Implement GetBulkRequest and a getSubtree function which utilises it. #44
Conversation
|
|
||
| // Helper to check whether `oid` in inside the tree rooted at | ||
| // `root` or not. | ||
| var inTree = function (root, oid) { |
There was a problem hiding this comment.
Why the move and reindent of this function?
There was a problem hiding this comment.
because inTree() now is used by both the pre-existing getSubtree() and the new getSubtreeBulk().
| // max-repetitions is the number of next variables that should be fetched | ||
| pkt.pdu.error = 0; | ||
| pkt.pdu.errorIndex = options.max_repetitions; | ||
| pkt.pdu.type = 5; |
There was a problem hiding this comment.
This should probably be a constant instead
There was a problem hiding this comment.
it wasnt in getNext(). Would you want me to change both?
|
Unfortunately I have not written formal tests for this - but we use it and it doesn't seem to fail against our switches (50+ device types). Perhaps we should discuss whether or not we even want a getSubtreeBulk - or perhaps the current getSubtreeBulk should replace the existing getSubtree. Is it justified to have both implementations? |
|
There should probably be only one getSubtree, that uses the best available method. If there are no compatibility issues with always using getBulk we should probably do that? if we expect issues, perhaps there could/should be a flag to fall back to normal getNext etc? |
|
OK - let me see when i find time to get this into shape. |
|
I think this is it. |
|
in the pas few weeks i have developed a healthy portion of skepticism, that just having one getSubtree is the way to move forward. perhaps i should redo this with a getBulkSubtree as there is equipment out there that may not support getbulkrequest... the lack of test for the getbulkrequest is also still a problem. |
|
So, eh, I'd obviously forgotten all about this (sorry!). I'm fine with either. Probably I'd suggest going with this as the one and only getSubTree, and when it becomes a clear issue that there is equipment that doesn't support this that we should work with - (re)introduce a getSubtreeCrappily that does it without bulk... |
|
i actually have equipment, which has a configuration variable which allows to enable bulk requests. so my getSubtree did not work until i enabled bulk requests in the configuration of the device. wrt tests, snmpjs does not support getBulkRequest, so all the getSubtree tests now fail. I am not sure what a good way forward is here... |
Requesting feedback for this implementation of GetBulkRequest.