RIPE hackathon go-netconf additions and modifications

[DavidJohnGee]

A large set of contributions here which break the current APIs.

Main features:

• Added a driver/abstraction mechanism that ensures each transport type exposes a set of common methods. This makes usage easier and removes poking about down at the session/transport levels for each driver. Also removes the current issue of naming each transport mechanism with different names.

• Port field is now exposed for SSH. This was previously fixed to 830.

• Added a client for binary users which consumes the driver mechanism.

• Modified some of the underlying types so they can be exposed at a higher level.

• Demo code currently hosted on different repo. Code that points to "ripe-stackstorm" or "arsonistgopher" will point to "juniper/go-netconf" if these changes are approved.

Check the client in the examples directory to see the total effect of the changes.

[GIC-de]

Hi David,

thanks for this great contribution. I will try to review and test this as soon as possible.

Regards GIC

[arsonistgopher]

Ok, good point. Addressed here 1066e8c https://github.com/Juniper/go-netconf/pull/65/commits/1066e8c62ff739d8296a17d307d081c61a821be2.

Any actions targeting the data store other than with Lock() and Unlock() will be sent through SendRaw() method. These are just helper funcs.

Thanks, D

On 26 Jun 2018, at 16:35, Christian Giese notifications@github.com wrote:

@GIC-de commented on this pull request.

In drivers/junos/junos.go https://github.com/Juniper/go-netconf/pull/65#discussion_r198194115:

• session "github.com/arsonistgopher/go-netconf/session" +)

• +// DriverJunos type is for creating a Junos based driver. Maintains state for session and connection. Implements Driver{} +type DriverJunos struct {

• Timeout time.Duration // Timeout for SSH timed sessions
• Datastore string // NETCONF datastore
• Session *session.Session // Session data +}

• +// New creates a new instance of DriverJunos +func New() *DriverJunos {

• return &DriverJunos{} +}

• +// SetDatastore sets the target datastore on the data structure I would not store the datastore (e.g. running or candidate) as part of the driver as this is information corresponds to the RPC. Comparing the candidate and the running configuration requires two RPC and with this approach you would like to do a SetDatastore before each of them.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Juniper/go-netconf/pull/65#pullrequestreview-132080419, or mute the thread https://github.com/notifications/unsubscribe-auth/AcR2A9eUX_DpdShvRV_7sb6y3VMe9YFNks5uAlTLgaJpZM4U3yge.

[arsonistgopher]

The approach here is to have the same experience with any driver, now or future. Current implementations feels super clunky and not easy to consume.

Also with regards to breaking existing implementations, it's entirely expected and I was hoping not to read this given the notice below from the go-netconf current README file.

Note: this is currently pre-alpha release. API and features may and probably will change. Suggestions and pull requests are welcome.

So either it's no longer pre-alpha release and thus remove the notice, or it is pre-alpha and let's not take the minor changes and extensive additions off the table.

I'm not precious over either. Let's be accurate with what it is today. The purpose of these changes is to make it easily consumable.

Thanks, D

On 26 Jun 2018, at 16:22, Christian Giese notifications@github.com wrote:

@GIC-de commented on this pull request.

We have to be careful because this change is completely breaking all existing implementations using this client library. I’m also not sure if this abstraction is required as we have currently just two drivers (ssh and junos local) and based on my experience with python ncclient, I do not expect further transport drivers in future.

I think that the usage of this library becomes also more complex, but these are just my two cents.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Juniper/go-netconf/pull/65#pullrequestreview-132073852, or mute the thread https://github.com/notifications/unsubscribe-auth/AcR2A5bgMDCscVWMEl62W_LOVTQK5c4Rks5uAlGkgaJpZM4U3yge.

[DavidJohnGee]

Closing PR for many reasons. The changes introduced inflict too many changes in a single atomic unit and we can spend a year here justifying them. It's a waste of everyone's time so will save the energy.

Best of luck for the future with this package.