Closed nemith closed 1 year ago
@rumenvasilev, @GiacomoCortesi
I agree with the proposal, I would probably rather pass the GetConfig etc. objects in the Call method.
I have an additional comment, there are circumstances in which we want to subscribe to a particolar netconf stream for receiving notifications.
The consumer of the library shall be able to receive these notifications when they arrive, in my fork I did that by creating a buffered notifications channel in the session object. The consumer can therefore listen on the channel and get back the notifications as soon as they are received.
So I was wondering if you are thinking about this possibility and whether or not you want to include it in the first beta release.
I agree with the proposal, I would probably rather pass the GetConfig etc. objects in the Call method.
This was my original design but decided to go with the session model but now that we’ve started to roll this out in prod it seems better to just have objects.
I have an additional comment, there are circumstances in which we want to subscribe to a particolar netconf stream for receiving notifications.
We don’t have an immediate need for notifications but I did want to support it. I think it should be able to be supported in the first beta release.
Instead of a buffer I would like to add a handler function. If you wanted you could have a handler that utilized a buffer. If the notification function is nil will will just drop the notifications.
Notification support could be added later without API changed (just additions)
I see, since I'm extensively using netconf notifications it would be a very nice addition for the project I'm working on.
So if you are willing to get any contributions on that I'd be glad to help.
Potential example of doing an RPC with the proposed call method
func main() {
s := netconfig.Open(t)
reply, err := s.Call(context.Background(), netconf.GetConfig{Datastore: netconf.Running})
if err != nil {
return er
}
if reply.Err() != nil {
return reply.Err()
}
var configReply netconf.ConfigReply
if err != reply.Decode(&configReply); err != nil {
return err
}
fmt.Println(configReply.Config)
}
@GiacomoCortesi
So if you are willing to get any contributions on that I'd be glad to help.
Contributions are welcome, but for new features a proposal about the design would probably be beneficial before you code it up.
Closed with #60
Problem
Right now there are two ways to issue a rpc request. There is a low level
Do
method which takes aRPCMsg
and returns aRPCReplyMsg
and there is a higher levelCall
request that takes in a request and response asany
and does xml marshaling directly on them. It also converts errors to Go errors.Both have some issues
Do
:innerxml
field on the message. What is the point in exposing the entire message. You can set aMessageID
but it would be overwritten on send.Call
:Errors are converted to Go errors, but errors in NETCONF have two severity levels: "warning" and "error". Raising warning's as Go errors seems wrong (and is what is done today). We can filter out the warning's but then they are inaccessible and your forced to use
Do
(which may not be a bad thing).On top of that some NETCONF servers like Junos like sending informational items as warnings when everything is ok.
Proposal
Combine both
Do
andCall
into a single method calledCall
with the following signature:Add a method to
Reply
(just renamed RPCReplyMsg) to xml decode a response.Note this may not be needed as it will just be a thin wrapper around xml.Decode?
Unexport
RPCMsg
(and probably just rename torequest
). There is nothing much that an end-user needs here.Have an option (on
Session
or onCall
?) for handling error messages. Can allow the user to determine if we should convert all error messages to Go errors, Just errors with severity oferror
, or none.Other considerations
I want to get this done before cutting a beta to keep the core api as stable as possible.
Given this API should we still have the core RFC6241 operations as methods on (s *Session) like GetConfig, etc? Or should they just be objects that are sent via
Call
?