-
Notifications
You must be signed in to change notification settings - Fork 41
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
External Integrations #398
Conversation
I looked at what you'd done and good idea with the config information but I'm not sure how that'd work for the configuration information though as that's technically static in most cases and without the ExternalIntegration being defined at time of Job run I'd be curious how it'd show data. Have you tested that bit? |
Without ExternalIntegration defined, this should work as before. If it's defined and specified as a job input argument, it takes precedence over PLUGIN_CONFIG values. Hope this is what was requested @cmsirbu Have tested without real job run, just to create an integration, and call the job with it. It reads the data as expected. Would be great if someone with access to Arista CVP could test this. |
The general idea was to avoid making this a breaking change - which means that if someone upgrades to the next release of SSoT and uses the PLUGIN_CONFIG method, their setup will continue working without any change. |
I'm talking about the Config Information that shows on the DataSource/DataTarget pages. I'm pretty sure that's read at App load and isn't able to be dynamically updated by the ExternalIntegration selection. |
I've refactored, how the configuration is gathered for CloudVision. There is a new optional key in |
Can you provide an invoke command for the script you mentioned in your PR description? I think this would aid development in the future |
Would rather do this in a separate PR not to block this one. |
…6-external-integrations
…lj-napps-282-jobs-script
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.
Few things to address but this looks good in general.
Add Demo Job CLI Script
Co-authored-by: Justin Drew <2396364+jdrew82@users.noreply.github.com>
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.
LGTM!
Closes: NaN
What's Changed