Prepare for integrations and add Infoblox integration #134
Conversation
|
This PR is ready for review now. Integration is not tested as I do not have access to any Infoblox instance. Commit 82b006a contains changes to files copied from nautobot-plugin-ssot-infoblox. |
| @@ -362,11 +362,11 @@ Once the dependencies are resolved, stop the existing containers, rebuild the Do | |||
|
|
|||
| ### Installing Additional Nautobot Plugins | |||
|
|
|||
| Let's say for example you want the new plugin you're creating to integrate into Slack. To do this, you will want to integrate into the existing Nautobot ChatOps Plugin. | |||
| Let's say for example you want the new plugin you're creating to integrate into Nautobot SSoT. To do this, you will want to integrate into the existing Nautobot SSoT Plugin. | |||
There was a problem hiding this comment.
I think this would be a specific example of a tool to integrate with, like "Infoblox", shouldn't it?
There was a problem hiding this comment.
This was probably copied from ChatOps plugin without changing the text, I just fixed it.
There was a problem hiding this comment.
Related to Slack, it's not an Integration, it's Chat Provider. There is no similar entity in SSoT App, that's why I changed Slack -> Nautobot. Any suggestions welcome.
nautobot_ssot/models.py
Outdated
| def _add_integrations(this_module): | ||
| for module in each_enabled_integration_module("models"): | ||
| for attr_name in dir(module): | ||
| attr = getattr(module, attr_name) | ||
| if isinstance(attr, type) and issubclass(attr, models.Model) and attr is not models.Model: | ||
| logger.debug("Adding model %s from %s", attr_name, module.__file__) | ||
| if attr_name in this_module: | ||
| raise ValueError( | ||
| f"Duplicate model name: {attr_name} from {module.__file__} class {attr.__class__.__name__}" | ||
| ) | ||
| this_module[attr_name] = attr | ||
|
|
||
|
|
||
| _add_integrations(globals()) |
There was a problem hiding this comment.
Can we maybe solve this using some kind of registration pattern that allows us not to mess with globals? It looks like a little too much magic for my taste 😃
If not, I would at least appreciate a docstring and some comments here.
There was a problem hiding this comment.
Other option is to use sys.modules, that needs to be imported. Using globals() seems better for me, as it doesn't need to be imported. Will add comments.
Co-authored-by: Leo Kirchner <Kircheneer@users.noreply.github.com>
Co-authored-by: Leo Kirchner <Kircheneer@users.noreply.github.com>
Co-authored-by: Leo Kirchner <Kircheneer@users.noreply.github.com>
Correct. |
development/nautobot_config.py
Outdated
| return os.getenv(f"NAUTOBOT_SSOT_{name}", default) | ||
|
|
||
|
|
||
| def _get_bool_env(name: str, default=False) -> bool: |
There was a problem hiding this comment.
Is there a reason you're not using the is_truthy helper function that's already imported from settings? I believe that's our standard pattern to use for determining the boolean of a particular value in nautobot_config.py.
There was a problem hiding this comment.
Several reasons :-)
is_truthyis usingstrtobool, that's deprecated in Python 3.10, will be removed.get_bool_envdoes more thatis_truthy(reads from env, prefixingNAUTOBOT_SSOThere), IMHO easier to implement locally using standard libs over custom implementation.
There was a problem hiding this comment.
I can rename these functions to _get_ssot_env or _get_ssot_bool_env, would that be more clear?
There was a problem hiding this comment.
I wasn't aware of the deprecation. If that's the case, it might make more sense to update that function in the core app to your method as I guarantee there are a lot of configurations in use out there using is_truthy that's going to break in the future. Better to fix that and continue using it instead of creating some replacement.
|
Tested and passed |
snaselj
changed the title
|
Resolved conflicts with develop. |
Closes #128 #115
TODO