New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for Nautobot 1.1.0 (Celery) #18
Conversation
e54585a
to
b6e37c8
Compare
username=data["username"], | ||
password=data["password"], | ||
secret=data["secret"], | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this code actually needed? Custom serialize/deserialize methods should only really be needed as special-case handling where things aren't trivially encodable to JSON. If we need this for even as simple a data model as this, I'm a bit concerned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two related aspects of the Credentials()
class:
- This simple data model does not require a class itself to store the data, agreed.
- This simple class was created to prevent credentials from being printed to stdout (by overriding
__repr__()
, which works just perfectly)
To keep # 1 and # 2 I believe we have to support these methods - when I did not do this, class' serialization has not been working correctly.
@@ -34,8 +33,30 @@ | |||
REQUEST_TIME = Summary("onboardingtask_processing_seconds", "Time spent processing onboarding request") | |||
|
|||
|
|||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts on the try/except pattern versus checking settings.VERSION
for <1.1 versus >= 1.1 and making a decision based on that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We did version based decision in https://github.com/networktocode/ntc-netbox-plugin-onboarding/blob/develop/netbox_onboarding/release.py
:
- it requires to parse the version with
package.version
, - code has several places with version-centric, conditional logic based on if statements,
Both approaches are similar in an end effect, with the first choice being a simpler/more readable in context of this particular change.
ed621df
to
0418e60
Compare
PR implements Celery support and provides Nautobot 1.1.0 compatibility:
nautobot.core.celery
.onboard_device_worker
function per worker type (Celery, RQ)Credentials
object receivednautobot_serialize
anddeserialize
methods as per Nautobot-core