charlestolley / python-snmp

A user-friendly SNMP library
MIT License
15 stars 3 forks source link

Assorted fixes #3

Closed itsjohannawren closed 1 year ago

itsjohannawren commented 1 year ago

Couple of fixes for things that cropped up while trying to use the library:

And while working on this I discovered the lack of a .gitignore. What I added is by no means complete, but it gets rid of the build and install cruft while working on the code.

charlestolley commented 1 year ago

Thanks for contributing! I'm not going to merge all these changes, for reasons I'll mention in a moment, but I will update the setup.cfg right away.

With regard to the .gitignore, I have been using my own .gitignore, which is configured to ignore itself. Maybe some time I'll add a proper one like you suggested, but I'm not sure it's a priority for me.

Finally, I would like to clarify that the reason the community comparison fails is that you are providing a str to the Manager() function, and it's meant to be a bytes type. I don't blame you for doing this, as I don't think I have that documented, and I have even considered changing it to use a str type, as I believe the textual convention behind the community string is compatible with unicode. It's important to me, however, to maintain very clear boundaries between str and bytes when dealing with OctetStrings, because that was one of my primary motivations for writing this library in the first place. The Python bindings for net-snmp, which is probably the most widely used C library for SNMP, treat the two as interchangeable (or did 5 years ago), which results in UnicodeErrors when handling OctetStrings with non-textual meanings. Anyway, all that to say, I appreciate your contributions, and, I will be, respectfully, declining this PR.

charlestolley commented 1 year ago

I decided to go ahead and change the type for the community argument, so as of this commit, Engine.Manager() expects a str for the community argument. I also made the same change to the get(), getNext(), getBulk(), and set() methods to SNMPv1Manager and SNMPv2cManager. Some day very soon, I will add proper documentation for these methods, and I may also add type annotations or type stubs. I've worked extensively on type annotations, but each time I try I find a new good reason to leave them out, so I"m not sure what I'll ultimately settle on.