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

Exclude tag requirements when loading DeviceType and InterfaceTemplate #671

Closed
wants to merge 1 commit into from

Conversation

kingfetty
Copy link
Contributor

Closes: #669

What's Changed

Remove tag filter when loading DeviceType and InterfaceTemplate into diffsync model.

To Do

  • Explanation of Change(s)
  • Added change log fragment(s) (for more information see [the documentation]

@kingfetty kingfetty requested review from chadell and a team as code owners January 17, 2025 15:51
@jdrew82 jdrew82 added type: enhancement New feature or request integration: ciscoaci Issues/PRs for Cisco ACI integration. type: bug Issues/PRs addressing a bug. and removed type: enhancement New feature or request labels Jan 20, 2025
@jdrew82
Copy link
Contributor

jdrew82 commented Jan 20, 2025

I'm not entirely sure this is a bug or an enhancement. This is changing the default behavior which makes me inclined to say that this should most likely be an optional setting for users. Can you refactor this to work from a setting in the PLUGINS_CONFIG for the App? The default behavior should be to use the Tag to filter the loaded DeviceTypes and InterfaceTemplates and only load all if the setting is True.

@jdrew82 jdrew82 added status: action required This issue requires additional information to be actionable type: enhancement New feature or request labels Jan 20, 2025
@kingfetty
Copy link
Contributor Author

I'm not entirely sure this is a bug or an enhancement. This is changing the default behavior which makes me inclined to say that this should most likely be an optional setting for users. Can you refactor this to work from a setting in the PLUGINS_CONFIG for the App? The default behavior should be to use the Tag to filter the loaded DeviceTypes and InterfaceTemplates and only load all if the setting is True.

I will work on refactoring as requested. I still believe it to be incorrect to filter DeviceType at all because it's something that will never be specific to ACI. The purpose behind the filtering for tags as it was explained to me is for integrations that are not ACI to not step on each other. It is highly unlikely that you will have DeviceTypes that are just created for ACI and no other purpose.

@pato23arg
Copy link
Contributor

pato23arg commented Jan 21, 2025

I'm not entirely sure this is a bug or an enhancement. This is changing the default behavior which makes me inclined to say that this should most likely be an optional setting for users. Can you refactor this to work from a setting in the PLUGINS_CONFIG for the App? The default behavior should be to use the Tag to filter the loaded DeviceTypes and InterfaceTemplates and only load all if the setting is True.

I will work on refactoring as requested. I still believe it to be incorrect to filter DeviceType at all because it's something that will never be specific to ACI. The purpose behind the filtering for tags as it was explained to me is for integrations that are not ACI to not step on each other. It is highly unlikely that you will have DeviceTypes that are just created for ACI and no other purpose.

@kingfetty without a filter/data ownership tag, the ACI SSOT will be "data owner" of all DeviceTypes. That means that ACI SSOT will have to declare every DeviceType in Nautobot. For example, if you declare a DeviceType manually, ACI SSOT will delete it during the next sync.
While for some users such behavior might be desired, most users want to still be able to define DeviceTypes manually or have others SSOTs able to define them. I agree with @jdrew82 that this could be an optional setting, but default behavior should remain the same.

@kingfetty
Copy link
Contributor Author

@kingfetty without a filter/data ownership tag, the ACI SSOT will be "data owner" of all DeviceTypes. That means that ACI SSOT will have to declare every DeviceType in Nautobot. For example, if you declare a DeviceType manually, ACI SSOT will delete it during the next sync. While for some users such behavior might be desired, most users want to still be able to define DeviceTypes manually or have others SSOTs able to define them. I agree with @jdrew82 that this could be an optional setting, but default behavior should remain the same.

Thank you for the clarification here, and a very good observation. I will withdraw this and kill the issue. I'll have to resort to manually tagging the DeviceTypes in my Nautobot Instance to get around this issue then.

@kingfetty kingfetty closed this Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration: ciscoaci Issues/PRs for Cisco ACI integration. status: action required This issue requires additional information to be actionable type: bug Issues/PRs addressing a bug. type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ACI loading requires tag on ACI agnostic objects resulting in duplication exceptions
3 participants