Skip to content

Conversation

gmicol
Copy link
Collaborator

@gmicol gmicol commented Sep 3, 2025

No description provided.

@gmicol gmicol self-assigned this Sep 3, 2025
@gmicol gmicol added the jira-sync Sync this issue to Jira label Sep 3, 2025
@gmicol gmicol requested a review from shrsr as a code owner September 3, 2025 17:53
@github-actions github-actions bot changed the title Add a new network resource module nd_local_user for local users on Nexus Dashboard v4.1.0 or higher Add a new network resource module nd_local_user for local users on Nexus Dashboard v4.1.0 or higher (DCNE-543) Sep 3, 2025
@gmicol gmicol force-pushed the add_network_ressource_nd_local_user branch from a4b3de7 to 6a9aefd Compare September 3, 2025 17:55
Comment on lines 218 to 221
identifier_key = "loginID"

nd = NDNetworkResourceModule(module, path, identifier_key)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if an object is identified by multiple keys?

sent = deepcopy(new_config)

for key in unwanted:
if isinstance(key, str):
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the reason for allowing unwanted list and str input?

# TODO: Add description to functions and clean the code
# TODO: Add exception cases
# NOTE: ONLY works for new API endpoints introduced in ND v4.1.0 and later
class NDNetworkResourceModule(NDModule):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like the approach of the class with generic methods that handle actions like merge etc

Copy link
Collaborator

@anvitha-jain anvitha-jain Oct 2, 2025

Choose a reason for hiding this comment

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

+1

from ansible_collections.cisco.nd.plugins.module_utils.nd import NDModule, issubset, sanitize


# TODO: Maybe make it an all new module class with request, exit_json, fail_json, etc...
Copy link
Collaborator

Choose a reason for hiding this comment

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

agree with this part, where we can also have a look at clean up current code

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

self.previous = []
self.proposed = []
self.sent = []
self.init_all_existing = self.query_obj(path=path, ignore_not_found_error=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

will this work for all endpoints? I noticed from NDO point of view for instance that the detailed view sometimes is deviating from the list view. Example would we templates and template summaries.

@gmicol gmicol force-pushed the add_network_ressource_nd_local_user branch 3 times, most recently from e38fee8 to df74fbf Compare September 5, 2025 19:48
@gmicol gmicol force-pushed the add_network_ressource_nd_local_user branch from df74fbf to fe89c61 Compare September 22, 2025 17:45
Copy link
Collaborator

@anvitha-jain anvitha-jain left a comment

Choose a reason for hiding this comment

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

Add comments to exaplain how the class works (or what is happening within the class)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira-sync Sync this issue to Jira

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants