Skip to content

Add plugins module.#203

Merged
pszulczewski merged 11 commits intodevelopfrom
add_plugins
May 11, 2023
Merged

Add plugins module.#203
pszulczewski merged 11 commits intodevelopfrom
add_plugins

Conversation

@pszulczewski
Copy link
Copy Markdown
Contributor

@pszulczewski pszulczewski commented Apr 27, 2023

Proposal of dynamic plugins module.

This approach for plugins is slightly different than nautobot core modules, which have all endpoints statically added in the code. Looking at the variety of plugins/apps ecosystem I think that following the same patterns for plugins endpoints (a module for each endpoint) would be impractical, tedious to develop and hard to maintain in the future.

This proposal assumes that, plugin(plugin base_url), endpoint and object attributes are given in by users in ansible playbook task. Object attributes are split into two module args:

  1. id where object identifiers like name, slug etc. for endpoint.get(**id) are given as k:v pairs
  2. attrs where extra k:v pairs are given for creating or updating an object

Error handling is done by backend pynautobot and error message is returned in ansible task msg key.

Still TODO if these modules are welcomed:

  • Extend d-compose nautobot with a plugin.
  • Add integration test for the module.

@pszulczewski pszulczewski force-pushed the add_plugins branch 4 times, most recently from 036a2a7 to 05e304a Compare April 28, 2023 10:07
@jvanderaa
Copy link
Copy Markdown
Contributor

Loving what I'm seeing here. The only thing I'm not quite sure on yet is the id field. Would be good to come up with another name here. I was thinking data, but that is a previous module implementation detail. So would like to get something else.

@pszulczewski pszulczewski linked an issue May 9, 2023 that may be closed by this pull request
@pszulczewski pszulczewski marked this pull request as ready for review May 9, 2023 10:41
@pszulczewski
Copy link
Copy Markdown
Contributor Author

Loving what I'm seeing here. The only thing I'm not quite sure on yet is the id field. Would be good to come up with another name here. I was thinking data, but that is a previous module implementation detail. So would like to get something else.

This part identifies an object. We can have, name, slug as well other non-standard names like in case of bgp plugin asn - see integration tests.
I was thinking that id - identifier is short and clear about what it is. data is ambiguous IMO.

If you don't like id please give a specific name to replace and I will change it.
For me it can even be something_else :)

@joewesch
Copy link
Copy Markdown
Contributor

joewesch commented May 9, 2023

I also like the idea here, thanks. This seems like something that should definitely be included.

I also don't like just id, but I could be ok with an alias. Also, since it can take multiple identifiers can we make it plural?

  identifiers:
    aliases: [ids]

Lastly, can we possibly add a test to make sure we can account for nested plugin endpoints like we did for pynautobot? That may require adding the data-validation-engine plugin as well to the test environment as that is the only plugin I am aware of that uses them.

@pszulczewski
Copy link
Copy Markdown
Contributor Author

Thanks for the comment @joewesch. I like that. 👍

Comment thread development/docker-compose.yml Outdated
Comment thread development/docker-compose.yml Outdated
Comment thread tests/integration/targets/latest/tasks/plugin_bgp_asn.yml
Comment thread tests/integration/targets/latest/tasks/plugin_bgp_asn.yml Outdated
Comment thread plugins/modules/plugin.py Outdated
Comment thread plugins/modules/plugin.py Outdated
@pszulczewski
Copy link
Copy Markdown
Contributor Author

Lastly, can we possibly add a test to make sure we can account for nested plugin endpoints like we did for pynautobot? That may require adding the data-validation-engine plugin as well to the test environment as that is the only plugin I am aware of that uses them.

I don't think we need that, as data-validation plugin has been standardized in the latest release 2.0.
see: nautobot/nautobot-app-data-validation-engine#29
Fixes API routes to be standardized (breaking change).
https://github.com/nautobot/nautobot-plugin-data-validation-engine/pull/29/files#diff-ad3b32d6e4e86b8fa1f80a5e6cad8f0eada338d217eb0f98c8d466de1eb6483d

@pszulczewski pszulczewski requested a review from joewesch May 10, 2023 09:50
@pszulczewski pszulczewski merged commit 6b0e19f into develop May 11, 2023
@pszulczewski pszulczewski deleted the add_plugins branch May 11, 2023 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New Module: Plugin Module

3 participants