Skip to content
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

Support accessing rows and walking tables #82

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

iwanb
Copy link

@iwanb iwanb commented Feb 20, 2018

Hi,

I have added a way to iterate over rows of a table, and also to access a complete row directly, i.e.:

for idx, row in manager.ifTable.iteritems():
    print(row.ifDescr)
    print(row.ifMtu)
for idx, row in manager.multiIndexTable["first"].iteritems():
    ...
row = ifTable[1]

It does slightly change the API (iteritems on a table would return the value of the first column before, I don't think it was intentional), but the normal iteration works the same.
Do you see other issues?

I am also thinking of adding a way to choose which columns you want to get in the walk (it's easy to do now), maybe as an argument to iteritems, what do you think?

While adding this I noticed that the SNMP session does not do a real iteration, all the values of the walk are put in a tuple before returning. IMO it would make sense to make the walk and walkmore methods of the SNMP Session class truly iterate as pysnmp fetches the results, then it will be possible to iterate over big tables efficiently. Do you know if that is easy to do? The pysnmp API you use now returns the full varbind in one go AFAIK, but I'm not very familiar with pysnmp.

Cheers,
Iwan

@coveralls
Copy link

coveralls commented Feb 20, 2018

Coverage Status

Coverage increased (+0.1%) to 88.656% when pulling 1b068bd on iwanb:tablewalk into b26a72a on vincentbernat:master.

@vincentbernat
Copy link
Owner

Thanks for the patch. I need some time to look at it.

As for using iterators, at the time it was written, PySNMP didn't provide an iterator to walk values. This may have changed. If not, we can also just walk ourselves instead of using the builtin method.

@iwanb
Copy link
Author

iwanb commented Feb 22, 2018

It looks like there was an issue with the CachedSession, the walk method was delegating to walkmore, which means it could also return too many results.
I solved it by calling walk, but I guess you could have an optimisation by using walkmore and filtering in CachedSession as well, so you can cache the extra results returned by walkmore.

I also introduced an issue by making walk an iterator, the iterator was cached instead of the walk result.

@vincentbernat
Copy link
Owner

I am a bit late into reviewing this as I am quite busy this month. I hope to be able to look at your PR next month.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants