-
Notifications
You must be signed in to change notification settings - Fork 39
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
Fix scripts so they no longer use Netbox.snmp_version #2429
Conversation
41dc570
to
d46511e
Compare
Codecov Report
@@ Coverage Diff @@
## master #2429 +/- ##
==========================================
- Coverage 53.34% 53.33% -0.02%
==========================================
Files 554 554
Lines 40315 40349 +34
==========================================
+ Hits 21507 21520 +13
- Misses 18808 18829 +21
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is mostly sound, except for a few questionable bits :)
d46511e
to
6d89cf5
Compare
c4a949b
to
fb0d969
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New issues introduced (and, still quite hard to figure out what changed since my last review when you force push or rebase your changes - I have to read everything all over again)
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something is definitely still broken here.
Using data from our lokalnav installation, these are my interactions with the navsnmp
command from this PR:
$ navsnmp buick
buick.lab.uninett.no has no valid SNMP configuration in NAV
This is patently false, the lab switch buick
does indeed have a valid SNMP profile.
Then there is one device whose name starts with ups
, which also has a valid SNMP profile, and yet, I get this:
$ navsnmp ups
Traceback (most recent call last):
File "/usr/local/bin/navsnmp", line 7, in <module>
exec(compile(f.read(), __file__, 'exec'))
File "/source/bin/navsnmp", line 98, in <module>
main()
File "/source/bin/navsnmp", line 38, in main
snmp_printer(netbox)
File "/source/bin/navsnmp", line 74, in snmp_printer
if profile.snmp_version == 1:
AttributeError: 'NoneType' object has no attribute 'snmp_version'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Things seem to work nicely now - I just have one question/nitpick :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well okay then :) Just a bit of rebase magic and you are good to go!
Also add functions to get snmp community or version from it
463bc8e
to
4f8d90e
Compare
4f8d90e
to
985eb3f
Compare
985eb3f
to
81f5ff6
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Fixes #2389.
Introduces a new function
get_preferred_snmp_management_profile
that return the profile with the highest snmp version, which then can be used to get the profiles snmp version or community.Also replaces the occurrences of
netbox.read_only
,netbox.read_write
andnetbox.snmp_version
using the new function everywhere expect in ipdevpoll. In ipdevpoll it is not clear yet how to do that since shadow classes are used there, which only copy the attributes, but not functions.