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

Obtain version on class init, allow compatibility with 2.x and 3.x APIs. #151

Merged
merged 2 commits into from
May 22, 2021

Conversation

jasonbarbee
Copy link
Contributor

Was looking for a few ISE calls to support a NetBox plugin for ISE and this worked well for adding and deleting endpoints, and I started thinking about 3.x support.

There is a workaround suggested on the Issue thread, but it requires modifying the library and maintaining a fork of the code vs the Pypi library, and, it doesn't have backward compatibility which I would like.

The idea is the class will initialize with a slightly longer timeout (changing from 2 -> 4 seconds) to obtain the version from the ISE API. If it succeeds, it stores a short version string on the class, if not, it's just empty ''.
If this direction is approved, this could determine how to obtain backwards compatibility and call functions differently and automatically on 2.x vs 3.x.
Tag: Issue #148 ISE 3.x support.

@joshand
Copy link
Contributor

joshand commented Apr 28, 2021

I like the idea, although I'm not a fan of the mandatory nature of this. I'd like to be able to opt out of this version check (or move it only to calls where version matters). For example, I use this library for TrustSec elements in ISE, and none of those APIs have changed in a breaking way. So forcing the additional loading time isn't something that I enjoy.

@jasonbarbee
Copy link
Contributor Author

I'm not sure why I changed that timeout. It certainly doesn't take 4 seconds. I profiled the code performance.
Class initialization and getting the version only takes 200ms on average to an ISE server 50ms RTT away.
I pushed my branch back down to 2 seconds, which was the current release's timeout.

The change is just a 200ms sanity check on initialization to verify if the API is working - rather than writing guards deeper in your code.

One more benefit doing this in class init you immediately know if the ISE object is valid. If the version is populated, you know the class is primed and ready to use.

Initialization will also be helpful in HA deployments too. The primary node ERS API is read/write, but the secondary node ERS API is read-only. If there is a failover event, the library could use initialization to determine the active node for ERS, and redirect the call to the current primary. That's outside this issue, but a great case for heading down the path of active initialization rather than passive.

@falkowich
Copy link
Owner

Hi, great PR!
Going to check it out this week.
Real life work is taking a toll for the moment :)

--
Regards Falk

@falkowich
Copy link
Owner

Arrh, I must reinstall my ISE dev env.
Both 2.7 and 3.0..
So this is this Friday evening fun :)

@falkowich
Copy link
Owner

I got some problems with this if the user that connects to ers don't have a admin account to webui.
Is there some way around that?
Or am I misunderstanding anything..

And sorry bout the mess with the merge / revert :)

--
Regards Falk

@falkowich
Copy link
Owner

My fault, if it is ok for you. Can you recommit the PR?

--
Regards Falk

@jasonbarbee
Copy link
Contributor Author

Ok... @falkowich I'm sorry, I've been on vacation, catching up. I'm a little lost. It looks like it was committed, then it was reverted, and both issues are marked closed. I'm not sure what you'd like me to do - perhaps resubmit a fresh PR?

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

Successfully merging this pull request may close these issues.

3 participants