Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define napalm port and transport for each platform #2029

Closed
ChrisdAutume opened this issue Apr 16, 2018 · 5 comments
Closed

Define napalm port and transport for each platform #2029

ChrisdAutume opened this issue Apr 16, 2018 · 5 comments
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application

Comments

@ChrisdAutume
Copy link

Issue type

[X ] Feature request
[ ] Bug report
[ ] Documentation

Environment

  • Python version: 3.5.4
  • NetBox version: 2.3.2

Description

Working with a variety of network platform os, we usually don't use the default port and transport. For now, it can only be changed using global parameter NAPALM_ARGS witch apply to all devices. This can be very problematic for exemple when working with NX-OS api (http/https/ssh) and Junos (netconf).

I suggest to add an optional specification of these two parameter in the platform create/edit page, the parameter will be merged with NAPALM_ARGS before napalm use.

@bdlamprecht
Copy link
Contributor

I agree that this option ideally would not be something that is globally defined, however, there are other optional_args that can be sent to NAPALM which (I believe) don't fit within your current change to the model for platform.

For example, all of the optional_args that NAPALM offers are shown here. A few off the top of my head that would be important (besides port and transport) are secret and config_lock.

Unfortunately, I don't know of a solution that would enable all of this added functionality while at the same time keeping things straightforward and uncomplicated to use when all you need are the defaults.

Just to be clear, I'm not dismissing the need, I'm merely adding a few thoughts on the issue at hand so as to "get it right" without going through several changes.

@ChrisdAutume
Copy link
Author

Well, I agree that it can be good to handle all parameter. I didn't find a good idea to do it yesterday.

Maybe the use of a napalm_optional_args in the platform model with serialisation/json and dynamic form can be a better proposition. With this setup, we can use planty of parameter or none if we only need the default, but it's less sexy.

@jeremystretch jeremystretch added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Apr 18, 2018
@jeremystretch
Copy link
Member

Let's avoid hard-coding any fields for specific NAPALM arguments, as there are quite a few today and there will likely be more in the future. A more robust approach might be to introduce a single napalm_args JSON field to store arbitrary per-platform arguments.

@amuckart
Copy link

I second the need for a per-platform access configuration. In a perfect world it would be nice to be able to define 'templates' of custom arguments to NAPALM (as free text to avoid having to track changes in NAPALM) and then assign those to devices or device templates in Netbox.

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application API change and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Jun 29, 2018
@jeremystretch
Copy link
Member

This has been implemented in 35d58d2

@lock lock bot locked as resolved and limited conversation to collaborators Jan 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants