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

Make compatible with Napalm 3 #30

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lukasjuhrich
Copy link

If I understand correctly, the problem with the MTU is that it is associated to a VLAN and not an interface (at least in our OS6860-P48) – obtainable via show vlan.

These parameter stubs are nonetheless required to match the current API and not throw any ImportErrors due to the compat access.

@lukasjuhrich
Copy link
Author

Compatability might be a problem; I see two options:

  • Try to support both the older and the newer NAPALM versions by applying the changes conditionally (I have no Idea how to go about the differing function signatures tho)
  • Drop support for the older NAPALM versions in a newer release (nobody forces people to upgrade napalm-aos anyway)

I also researched when the changes made it into napalm:

  • Demanding Python 3.6+ support: Napalm 3.0.0
  • Adding sanitized to get_config: Napalm 3.0.0 as well (See this PR)
  • Adding longer to get_route_info: No idea
  • MTU support: Napalm 2.5.0

@lukasjuhrich lukasjuhrich changed the title Fix some incompatabilities with current NetworkDriver API Make compatible with Napalm 3 Oct 18, 2020
@lukasjuhrich
Copy link
Author

A workaround independent of this PR would be to modify the given requirement from napalm>=2.3.0 to napalm>=2.3.0,<2.5 in order to exclude the newer incompatible releases. Could this be done in a patch release?

@FlorianHeigl
Copy link

given the current state it might be best to just foward-port and forget compatibility issues.
there's no practical way to use napalm_aos at the moment with napalm/netmiko of considerable age.
if someone wants to use it with the old versions, because that's what they use - they still can.
but it is not going to work out excluding any users that are not stuck on old versions.

did anyone ever work with you / consider pulling in the PRs here?

@lukasjuhrich
Copy link
Author

Hi @FlorianHeigl , I haven't received any communication. I just recall trying to use it with our network hardware and fixing it, but I cannot give any more details due to lack of memory.

I have no problems with forward-porting, it should just follow good etiquette and increment the major version number as to prevent uninentional updates on the side of users.

@lukasjuhrich
Copy link
Author

lukasjuhrich commented Mar 10, 2021

I just noticed that it got out of date and people may be waiting for me to rebase.
Well, I didn't look at the PR because I didn't receive any feedback; I am just a python developer and know little about Napalm, so some sort of feedback feedback is definitely necessary. Not that this became some sort of deadlock :D

I can try to rebase it, but I would have to backtrack where we use that on our side to test it to some degree; I can't promise I find spare time for that.

@jefvantongerloo
Copy link

I used this PR to test the compatibility with my PR (#34), because get_vlans() parent function is only present starting from Napalm 3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants