search

calmh / node-snmp-native

Native Javascript SNMP library for Node.js
MIT License
252 stars 65 forks source link

Implement GetBulkRequest and a getSubtree function which utilises it. #44

Open bangert opened 7 years ago

bangert commented 7 years ago

Requesting feedback for this implementation of GetBulkRequest.

bangert commented 7 years ago

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?

calmh commented 7 years ago

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?

bangert commented 7 years ago

OK - let me see when i find time to get this into shape.

bangert commented 7 years ago

I think this is it.

bangert commented 7 years ago

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.

calmh commented 7 years ago

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...

bangert commented 7 years ago

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...