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

more than 255 arguments #15

Closed
cqhan777 opened this issue Oct 29, 2017 · 8 comments
Closed

more than 255 arguments #15

cqhan777 opened this issue Oct 29, 2017 · 8 comments

Comments

@cqhan777
Copy link

cqhan777 commented Oct 29, 2017

codegen/pysnmp.py

Can generate python script with more than 255 arguments, which make it not working.
This happened to me on ObjectGroup().setObjects() call. But it can happen on other setObjects() calls as well.
Worked around in my case by editing the generated .py file to assign the large tuple directly to ObjectGroup.objects, which takes a tuple and happened to be public member.
RARITAN-PX2-PDU2-MIB.txt
Attached is the MIB file that would cause problem. Line 71 of generated .py contains > 255 arguments.

@etingof
Copy link
Owner

etingof commented Nov 2, 2017

Thank you for raising this issue!

The fix is here, I'd appreciate your testing and reporting back how it worked for you. So I could merge that branch and make a release.

@cqhan777
Copy link
Author

cqhan777 commented Nov 2, 2017 via email

@cqhan777
Copy link
Author

cqhan777 commented Nov 3, 2017

Hi Ilya,

The fix works in my case, but I don't why it would work. From the fix, it seems that cascading setObject() would result in only the last setObject() call being effective.

The source code for ObjectGroup class in SNMPv2_CONF.py looks like:

class ObjectGroup(MibNode):
objects = ()
description = ''

def getObjects(self):
    return getattr(self, 'objects', ())

def setObjects(self, *args):
    self.objects = args
    return self

def getDescription(self):
    return getattr(self, 'description', '')

def setDescription(self, v):
    self.description = v
    return self

So calling the setObjects() twice would void the first call. Did I miss something here?

Regards,
Kevin

@etingof
Copy link
Owner

etingof commented Nov 5, 2017

Hey Kevin,

The fix works in my case, but I don't why it would work. From the fix, it seems that cascading setObject() would result in only the last setObject() call being effective.

You are right, that's my bad!

I've reworked the pysmi implementation (564f845) and added the incremental objects adding mode to pysnmp (commit 1739070fb0702451a34608cf0c39c44bf1e09514).

I wonder if you could re-test those? ;-)

@cqhan777
Copy link
Author

cqhan777 commented Nov 5, 2017

Hi Ilya,

I see one potential problem: Line 412 (and others)

append=False)
wonder it should be
append=True

Also I wonder if you are changing the pysnmp code, why not just add a new funciton, something like
setObjectList( object_tuple), where the tuple object is the argument instead of expand it.

    def setObjectsList(self, list_args):
        self.objects = list_args
        return self   

Then in codegen/pysnmp.py

        objStr = ', '.join(objects)
        outStr = name + ' = NotificationGroup(' + oidStr + ')' + label + '\n'
        if getattr(mibBuilder, 'version', 0) < (4, 4, 2):
            outStr += name + '.objects = (' + objStr + ')\n'
        else:
            outStr += name + 'setObjectList((' + objStr + '))\n'

@etingof
Copy link
Owner

etingof commented Nov 5, 2017

I see one potential problem: Line 412 (and others)

Indeed! Fixed in 1d7ca20.

Also I wonder if you are changing the pysnmp code, why not just add a new funciton, something like
setObjectList( object_tuple), where the tuple object is the argument instead of expand it.

Yes, this is definitely another legitimate way.

I'm leaning towards kwargs because I expect that some other need for customizing those .set*() setters behavior may come up. Creating combinations of functions like setObjectList/setObjectSomethingElse might become tedious by some point.

Does it make sense?

Thanks!

@cqhan777
Copy link
Author

cqhan777 commented Nov 6, 2017

I'm OK with it. Thanks for fixing the problem!.

@etingof
Copy link
Owner

etingof commented Nov 6, 2017

Merged, thank you!

@etingof etingof closed this as completed Nov 6, 2017
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

No branches or pull requests

2 participants