falkowich/pyise-ers

add_devices - ISE 3.x

Closed this issue · 8 comments

Hi there,
First, thanks for this great library, it's useful!

After that, I would like to comment that the function "add_device" doesn't work for ISE 3.0. I found that the parameter "dev_group" isn't necessary.

These lines are in https://github.com/falkowich/ise/blob/master/ise.py#L1289, https://github.com/falkowich/ise/blob/master/ise.py#L1305, and https://github.com/falkowich/ise/blob/master/ise.py#L1343.

Regards,
MO.

Nice, going to install a 3.0 in lab and try it out.
Thanks for the report!

--
Regards Falk

Hi, when finally installing a 3.0 in homelab.. puh what a beast :)

This started with 2.7 I think. I tried that one too.
So the "plan" is to make "device_group" optional in the function.

Don't really know how I will make it backwards compatible. But perhaps the easier way is to default on < 2.7 (or what version that change was in) and then do a config option for what ISE is the host..

--
Regards Falk

If you change the function definitions like this:

    def add_device(self,
                   name,
                   ip_address,
                   radius_key,
                   snmp_ro,
                   dev_group=None,
                   dev_location=None,
                   dev_type=None,
                   description='',
                   snmp_v='TWO_C',
                   dev_profile='Cisco',
                   tacacs_shared_secret=None,
                   tacas_connect_mode_options='ON_LEGACY'
                   ):

This should still allow existing code to work. Then you just need to add a relevant code block in the function to test and split old vs new functionality... "if dev_group". Since you can't have a =None in the middle, you'll probably have to add some sanity checking to make sure dev_location and dev_type are values that they need to be.

Maybe something like this?

if not dev_group:
    # code specific to ise 3.0
    pass
elif not dev_location or not dev_type:
    # error if these are actually required parameters
    pass
else:
    # original code here
    pass

I don't use this part of ISE right now, but I do have ISE 2.4, 2.6, 2.7 and 3.0 in my lab. Happy to help with any kind of regression testing if you have some thoughts on how you want to fix this (I'll probably need some detail on what I need to configure in ISE to actually test this).

Ouch, I really messed up with that merge @jasonbarbee..

@joshand I would really be glad with some help with regression testing and thoughts :)
I have now pushed a new 2.7 and 3.0 on my poor kvm server @ home :)

And #152 made dev_group optional with dev_group=None.

But the PR that @jasonbarbee made that checks versioning is really interesting too.

--
Regards Falk

@mortiz-code - Can you test out the master branch and see it things works as expected?

--
Regards Falk

iwics commented

@falkowich thanks for the fix. Any chance to update the PyPi soon?

I will check this out as soon as #164 is done

--
Regards Falk

This will be included in PyPi release pyise-ers 0.2.0