Skip to content
This repository has been archived by the owner on Sep 25, 2024. It is now read-only.

add_devices - ISE 3.x #148

Closed
mortiz-code opened this issue Feb 16, 2021 · 8 comments
Closed

add_devices - ISE 3.x #148

mortiz-code opened this issue Feb 16, 2021 · 8 comments

Comments

@mortiz-code
Copy link

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.

@falkowich
Copy link
Owner

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

--
Regards Falk

@falkowich falkowich self-assigned this Feb 16, 2021
@falkowich falkowich added the bug label Feb 16, 2021
@falkowich falkowich added this to the 0.2 milestone Feb 16, 2021
@falkowich falkowich changed the title add_devices add_devices - ISE 3.x Feb 16, 2021
@falkowich
Copy link
Owner

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

@joshand
Copy link
Contributor

joshand commented Mar 8, 2021

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

@falkowich
Copy link
Owner

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

@falkowich
Copy link
Owner

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

--
Regards Falk

@falkowich falkowich modified the milestones: 0.2, 0.1.3 May 23, 2021
@iwics
Copy link

iwics commented Jun 8, 2021

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

@falkowich
Copy link
Owner

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

--
Regards Falk

@falkowich
Copy link
Owner

This will be included in PyPi release pyise-ers 0.2.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants