Move <rpc-reply> unpacking to RPC client#215
Draft
Nothing4You wants to merge 1 commit intoczerwonk:mainfrom
Draft
Move <rpc-reply> unpacking to RPC client#215Nothing4You wants to merge 1 commit intoczerwonk:mainfrom
Nothing4You wants to merge 1 commit intoczerwonk:mainfrom
Conversation
Nothing4You
commented
Jun 14, 2023
pkg/rpc/client.go
Outdated
Comment on lines
+75
to
+82
| var reply *rpcReply | ||
| err = xml.Unmarshal(b, &reply) | ||
|
|
||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| b = reply.Body |
Contributor
Author
There was a problem hiding this comment.
this section would be completely skipped when using a netconf transport
Owner
|
I quite like the idea. Also makes the existing code more readable due to less chaining. Please go ahead if you like. Happy to review and merge when done. |
This makes it easier to implement alternative transports, such as netconf, which may already unpack <rpc-reply> on transport level before handing the contained response to the RPC client.
Nothing4You
force-pushed
the
unpack-rpc-reply-on-transport
branch
from
10dd026 to
acca978
Compare
|
Would really like to have netconf support added into the exporter. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This makes it easier to implement alternative transports, such as netconf, which may already unpack
<rpc-reply>on transport level before handing the contained response to the RPC client.With https://github.com/nemith/netconf, we never see the
<rpc-reply>element in the first place, only its contents are passed on.This moves the unpacking logic to a central point and separates this change from the netconf implementation, which should result in smaller diffs to review.
Can you please tell me if you consider this a reasonable change?
If you do, I'll go ahead and convert the remaining places as well.