search

networktocode / ntc-netbox-plugin-onboarding

A plugin for NetBox to easily onboard new devices.
Other
245 stars 46 forks source link

Update to Napalm version 3.2.0 #114

Closed nniehoff closed 3 years ago

nniehoff commented 3 years ago

This PR is to update the onboarding plugin to Napalm 3.2.0. I had to update all dependencies with poetry hence the updates to poetry.lock.

nniehoff commented 3 years ago

I also did some cleanup for NetBox 2.9 (configuration.py) and pylint.

dgarros commented 3 years ago

Could we support both napalm 2 and napalm 3 ? if we don't have to enforce a specific version I think it's better. poetry by default limit

Looking at it more closely, I think our pyproject.toml is too restrictive right now, we shouldn't pin our dependencies to a specific version 

# from pyproject.toml
invoke = "^1.4.1"
napalm = "^3.2.0"

https://python-poetry.org/docs/dependency-specification/

glennmatthews commented 3 years ago

@dgarros If I'm reading that link correctly, ^1.4.1 in pyproject.toml means >=1.4.1, <2.0.0, which seems reasonable to me.

Agreed that it would be nice (if possible) to set the napalm dependency to something like >=2.5.0, <4.0.0 instead of requiring at least 3.2.0.

nniehoff commented 3 years ago

Could we support both napalm 2 and napalm 3 ? if we don't have to enforce a specific version I think it's better. poetry by default limit

Looking at it more closely, I think our pyproject.toml is too restrictive right now, we shouldn't pin our dependencies to a specific version

# from pyproject.toml
invoke = "^1.4.1"
napalm = "^3.2.0"

https://python-poetry.org/docs/dependency-specification/

AIUI right now we are basically saying for napalm we want versions >= 3.2.0 < 4.0.0. We could change this to something like:

napalm = ">= 2.5.0, < 4"

I think the problem is if version 4 comes out I don't think we want to automatically support that without testing so at the very least I think we need to keep the < 4 just to be safe. If we want to add support for multiple versions we will also need to adjust the tests for multiple versions.

mzbroch commented 3 years ago

Could we support both napalm 2 and napalm 3 ? if we don't have to enforce a specific version I think it's better. poetry by default limit

I agree, in general we're more focused on NAPALM facts structure and its content, than a specific NAPALM version.