Skip to content

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.

3 participants