-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
Fixed Length OID Processing Fixes #166
base: main
Are you sure you want to change the base?
Conversation
After some PEP 8 changes in etingof#243, the temporary variable name clashed with the name of the OID being processed. This change renames the temporary variable to fix this clash.
After the following commit, lextudio@4c71d45, the fixed length accessor functions for the Octet String class were changed to be PEP 8 compliant. This commit updates SNMPv2-SMI to use the PEP 8 compliant function calls.
I feel obliged to remark here that while your changes are an excellent improvement as far as I can tell, last week's commit 755e8a7 once again renamed the underlying attribute |
This updates the fixed length attributes of the INET-ADDRESS-MIB classes to conform to PEP 8 compliance requirements
This updates the MAC Address fixed length attribute to conform to the PEP 8 compliance changes
Update the SNMPv2-TM MIB to use the updated fixed length attribute to comply to PEP 8 requirements
Updated the fixed length attributes in the TRANSPORT-ADDRESS-MIB to conform to PEP 8 requirements
I just created a pull request in pysmi for this as well. This should update the unit tests as well as the js file from the mib parsing |
We decided to delay changes related to PEP 8 to a future release, so these two pull requests from @WakkaTrout will not be reviewed or merged until then. |
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.
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
pysnmp/smi/mibs/SNMPv2-SMI.py:1232
- [nitpick] The variable name fixed_length is inconsistent with the naming convention used in the rest of the code. It should be renamed to fixedLength.
fixed_length = obj.get_fixed_length()
BTW, this pull request has no test case to show what has been fixed. The information included in #141 is a good start, but not enough. |
I can add unit tests (my testing has been ad hoc testing with a remote device I put MIB's on (the device is running netsnmp)). I see the tests in the tests folder and I see them reference the LEXTUDIO-TEST-MIB (which I assume is hosted at https://mibs.pysnmp.com/asn1/ somewhere). Do I have the ability to edit this MIB to add various other tables for testing? If so, how can I go about this? |
@WakkaTrout The MIB documents behind https://mibs.pysnmp.com can be found in this GitHub repo, https://github.com/lextudio/mibs.pysnmp.com You can create pull requests there if you want to add your own test MIB files, or you want to modify LEXTUDIO-TEST-MIB to include some objects. |
Sorry for the long wait on this, I have been super busy and didn't manage to get around to this.
For context: This fixes the closed issue #141 that is still present in the main branch after some initial testing.
The following pull request restores the Fixed Length OID processing functionality that began to cause crashes for fixed length OID strings after PEP 8 changes were applied. Additionally, this updates the SNMPv2-SMI.py file to use the newer PEP 8 compliant function names.