-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add functionality for handling varbind errors in get command #262
Conversation
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports |
Test results 3 files 3 suites 52s ⏱️ Results for commit e940301. ♻️ This comment has been updated with latest results. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #262 +/- ##
=======================================
Coverage 99.03% 99.04%
=======================================
Files 50 50
Lines 6735 6750 +15
=======================================
+ Hits 6670 6685 +15
Misses 65 65 ☔ View full report in Codecov by Sentry. |
7f51422
to
33d036b
Compare
33d036b
to
c4c2900
Compare
1292d62
to
e940301
Compare
|
Quality Gate passedIssues Measures |
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 PR makes use of the new function in the
get
command, but should be used for all the different commands. That can be done in other PRs once this one is completed.
Not necessarily. In cases where the client code expects to fetch multiple values using a single operation, the client may be fine with some of the return values being errors. The client might want to keep the successfully retrieved values and make decisions about changing its queries for failed values.
This could also be a flag to a method that retrieves multiple values. The default could be to raise exceptions, but if the calling code wants to handle error values in custom ways, it could set a flag, asking the library to stay out of its business.
Also sneakily fixes
ErrorIndication
not inheriting correctly (an SNMP related exception should probably inherit from the exception specifically made to be a base exception for everything SNMP related...)
👍
Also2, there's quite a few SNMP specific exception classes now, thoughts on putting them in their own file?
Sure, I'd support that - but the likely cleanup here would be to split zino/snmp.py
into zino/snmp/__init__.py
and zino/snmp/errors.py
.
Work towards fixing #261 and #212
Adds function that takes an
ObjectType
and raises a relevant exception if it contains an error instead of a real value. I refer to this type of error as varbind errors since they are not defined viaerrorStatus
orerrorIndication
, but are defined as values inside the response varbindThis PR makes use of the new function in the
get
command, but should be used for all the different commands. That can be done in other PRs once this one is completed.To fully fix #212, some changes have to be made to the
juniperalarmtask
, including reverting some changes made in #231.Handling of the new errors should also be added.
NoSuchObjectError
is definitely relevant, but maybeNoSuchInstanceError
is too? Im not too sure about the difference about these two.. I'll leave @johannaengland to complete this taskAlso sneakily fixes
ErrorIndication
not inheriting correctly (an SNMP related exception should probably inherit from the exception specifically made to be a base exception for everything SNMP related...)Also2, there's quite a few SNMP specific exception classes now, thoughts on putting them in their own file?