search

napalm-automation / napalm-base

Apache License 2.0
32 stars 48 forks source link

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

Open ktbyers opened 7 years ago

ktbyers commented 7 years ago

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 commented 7 years ago

This needs to be pushed with:

https://github.com/napalm-automation/napalm-base/pull/213

I have tested with napalm-ios.

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

https://github.com/napalm-automation/napalm-ios/pull/124

dbarrosop commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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