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

Add support for transition of signatures (to ease migration of methods) #239

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

ktbyers
Copy link
Contributor

@ktbyers ktbyers commented Apr 19, 2017

Adds a new data structure TRANSITION_SIGNATURES that allows for easier migration of methods from old to new state.

napalm-base would be pushed first with the new signature.
Then all napalm-drivers would be pushed with new signature.
Then TRANSITION_SIGNATURES would be commented out (i.e. set to blank list until next migration of signatures occur).

@ktbyers
Copy link
Contributor Author

ktbyers commented Apr 19, 2017

This needs to be pushed with:

#213

I have tested with napalm-ios.

Once pushed, I will further regression test against this PR.

napalm-automation/napalm-ios#124


# Method signatures that are migrating from old to new state (temporary state)
transition = True
if transition:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand this :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes...that isn't very good...let me cleanup.

Copy link
Member

@mirceaulinic mirceaulinic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a nice solution IMO

@dbarrosop
Copy link
Member

dbarrosop commented Apr 21, 2017

Let's discuss it in the meeting next week. This has big implications so I'd like to discuss it and make sure everybody is on the same page before moving forward. My fear is that this basically opens the door to non-compatible code. You could install napalm and have two drivers with two different method signatures and write incompatible code. A solution could be to add *args, **kwargs to all signatures.

@ktbyers
Copy link
Contributor Author

ktbyers commented May 25, 2017

@dbarrosop @mirceaulinic Can we revisit this as a transition solution for migrating the API? Without something like this progress on these API changes grinds to a halt.

@dbarrosop
Copy link
Member

Yes, we should proceed with this. I have one comment though. Could we have a yaml file instead in the root of the repo where we put the signatures instead of buried in the code? The reason is to have it on a more visible place so we don't forget about them. It would be great also if we could trigger a huge warning during the tests just to annoy us so we don't grow lazy because of this :P

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

Successfully merging this pull request may close these issues.

3 participants