Skip to content
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

Add LCM plugin config. Fix 'infoblox_import_subnets` setting. #379

Merged
merged 2 commits into from
Mar 8, 2024

Conversation

progala
Copy link
Contributor

@progala progala commented Mar 7, 2024

Updates to dev nautobot_config.py:

  • Adds config for Nautobot Device Lifecycle Management plugin.
  • Fixes logic in infoblox_import_subnets setting.

@progala progala requested a review from a team as a code owner March 7, 2024 15:00
@jdrew82 jdrew82 added the integration: infoblox Issues/PRs for Infoblox integration label Mar 7, 2024
@jdrew82
Copy link
Contributor

jdrew82 commented Mar 7, 2024

So I'm a bit confused here. You're creating a list it looks like, but shouldn't that automatically be done if the value is a concatenated string with the split()?

@progala
Copy link
Contributor Author

progala commented Mar 7, 2024

Problem here is that the default value "" will result in "".split(","). This does not produce an empty list which breaks logic in https://github.com/nautobot/nautobot-app-ssot/blob/develop/nautobot_ssot/integrations/infoblox/diffsync/adapters/infoblox.py#L51

>>> "".split(",")
['']

@jdrew82
Copy link
Contributor

jdrew82 commented Mar 7, 2024

And you've got the same result with your suggestion:

>>> [x for x in "".split(",") if x]
[]

@progala
Copy link
Contributor Author

progala commented Mar 8, 2024

It's not the same. One is a 1-element list with an empty string [''], the other is a 0-element list []. bool(['']) evaluates to True, bool([]) evaluates to False.

@jdrew82
Copy link
Contributor

jdrew82 commented Mar 8, 2024

Fair point. Looks like it should work right with a passed string as typically expected too so I'm fine with merging this. Can we also get the documentation updated to match?

Copy link
Contributor

@jdrew82 jdrew82 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jdrew82 jdrew82 merged commit 3783655 into nautobot:develop Mar 8, 2024
15 checks passed
@jdrew82 jdrew82 mentioned this pull request Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration: infoblox Issues/PRs for Infoblox integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants