Inventory Workflow - Compute device related changes updated #293

md-rafeek · 2025-08-18T20:50:40Z

Type of Change

Bug fix
New feature
Breaking change
Documentation update

Description

As per the bug, when a new compute device was added to the inventory with only the required fields, it threw an error because the code did not separate compute devices. This has now been fixed.

Bug Fix:
The bug was raised when adding a compute device to the inventory with only the required fields. It failed earlier, but the issue is now resolved.

Root Cause (if applicable):
The code did not properly separate compute device types, so SNMP parameters for network devices were also being added to compute devices.

Fix Implemented:
The code was updated to correctly separate compute devices and network devices, and some cognitive complexity was reduced.

Enhancement: [Brief description of the improvement/enhancement made]
Enhancement Description: [Explain what was enhanced, why, and how]
Impact Area: [Mention which part of the system/codebase is affected]

Testing Done:

Manual testing
Unit tests
Integration tests

Test cases covered: [Mention test case IDs or brief points]

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules
All the sanity checks have been completed and the sanity test cases have been executed

Ansible Best Practices

Tasks are idempotent (can be run multiple times without changing state)
Variables and secrets are handled securely (e.g., using ansible-vault or environment variables)
Playbooks are modular and reusable
Handlers are used for actions that need to run on change

Documentation

All options and parameters are documented clearly.
Examples are provided and tested.
Notes and limitations are clearly stated.

Screenshots (if applicable)

Notes to Reviewers

madhansansel · 2025-08-25T09:26:47Z

plugins/modules/inventory_workflow_manager.py

+            devices_to_add (list): A list of devices to be added.
+
+        Returns:
+            object: An instance of the class with updated results and status.


I don't see any return values from the API.. Can you check and update?

Added the self as return

madhansansel · 2025-08-25T09:27:07Z

plugins/modules/inventory_workflow_manager.py

+            device_params (dict): A dictionary containing device parameters.
+
+        Returns:
+            dict: A dictionary containing parsed compute device parameters.


Not returning any values.. can you check and update?

Return not required because we are just parsing the param to to device_params also it is passing from outside. i have added device_params as return now

madhansansel · 2025-08-25T09:28:08Z

plugins/modules/inventory_workflow_manager.py

+                "DEBUG",
+            )
+            self.cred_updated_not_required.append(device_ip)
+            return


in Docstring you mentioned as
Returns:
dict: A dictionary containing parsed network device parameters for update.

But not returning anything from here..

updated as return device param, and playbook param

madhansansel · 2025-08-25T09:28:24Z

plugins/modules/inventory_workflow_manager.py

+            if not playbook_params["snmpRwCommunity"]:
+                playbook_params["snmpRwCommunity"] = device_data.get(
+                    "snmp_write_community", None
+                )


Don't we have any return value?

updated playbook param as return

…talyst-center-ansible-intg into inventory_workflow_manager

madhansansel · 2025-08-25T17:03:34Z

plugins/modules/inventory_workflow_manager.py

+                self.msg = "Required parameter 'snmpROCommunity' for adding device with snmmp version v2 is not present"
+                self.result["response"] = self.msg
+                self.log(self.msg, "ERROR")
+                return self


returning self here..but in else part.. it is not returning anything..

Actually return not required. but if the SNMP param not available for V2 version need to fail and exit. added the device_params as return below.

…talyst-center-ansible-intg into inventory_workflow_manager

md-rafeek

addressed Code review comments.

md-rafeek · 2025-08-26T03:36:23Z

plugins/modules/inventory_workflow_manager.py

+            device_params (dict): A dictionary containing device parameters.
+
+        Returns:
+            dict: A dictionary containing parsed compute device parameters.


Return not required because we are just parsing the param to to device_params also it is passing from outside. i have added device_params as return now

md-rafeek · 2025-08-26T03:47:19Z

plugins/modules/inventory_workflow_manager.py

+            devices_to_add (list): A list of devices to be added.
+
+        Returns:
+            object: An instance of the class with updated results and status.


Added the self as return

md-rafeek · 2025-08-26T03:51:25Z

plugins/modules/inventory_workflow_manager.py

+                self.msg = "Required parameter 'snmpROCommunity' for adding device with snmmp version v2 is not present"
+                self.result["response"] = self.msg
+                self.log(self.msg, "ERROR")
+                return self


Actually return not required. but if the SNMP param not available for V2 version need to fail and exit. added the device_params as return below.

md-rafeek · 2025-08-27T11:51:01Z

plugins/modules/inventory_workflow_manager.py

+                "DEBUG",
+            )
+            self.cred_updated_not_required.append(device_ip)
+            return


updated as return device param, and playbook param

md-rafeek · 2025-08-27T11:51:41Z

plugins/modules/inventory_workflow_manager.py

+            if not playbook_params["snmpRwCommunity"]:
+                playbook_params["snmpRwCommunity"] = device_data.get(
+                    "snmp_write_community", None
+                )


updated playbook param as return

madhansansel · 2025-08-28T18:47:26Z

plugins/modules/inventory_workflow_manager.py

+                device_params.pop("snmpPrivPassphrase", None)
+                device_params.pop("snmpPrivProtocol", None)
+
+            return device_params


It is still not clear. We are returning 'device_params', yet in the if statement, we are returning self. Could you please check and update the entire code?

This function was splitting from large function to small, this is function used to parse and remove unwanted values from "device_params" we are passing "device_params" as argument, any required values in param is missing then need to show failed return. the same updated.

…talyst-center-ansible-intg into inventory_workflow_manager

md-rafeek

Address the review comments

md-rafeek · 2025-08-29T11:20:40Z

plugins/modules/inventory_workflow_manager.py

+                device_params.pop("snmpPrivPassphrase", None)
+                device_params.pop("snmpPrivProtocol", None)
+
+            return device_params


This function was splitting from large function to small, this is function used to parse and remove unwanted values from "device_params" we are passing "device_params" as argument, any required values in param is missing then need to show failed return. the same updated.

madhansansel · 2025-09-01T04:36:30Z

plugins/modules/inventory_workflow_manager.py

+                self.log(self.msg, "ERROR")
+                self.fail_and_exit(self.msg)
+
+            while True:


Need to add sleep

Also add a code to come out of while, otherwise it will be in continuous loop

added the sleep and exit while statement.

md-rafeek

Addressed code review comments.

md-rafeek · 2025-09-01T11:51:36Z

plugins/modules/inventory_workflow_manager.py

+                self.log(self.msg, "ERROR")
+                self.fail_and_exit(self.msg)
+
+            while True:


added the sleep and exit while statement.

madhansansel · 2025-09-02T04:56:34Z

plugins/modules/inventory_workflow_manager.py

+                    self.log(self.msg, "ERROR")
+                    self.result["response"] = self.msg
+                    break
+


Add a log message before sleep..

Added the Sleep log message.

md-rafeek

Address Sleep log message.

md-rafeek · 2025-09-02T11:17:12Z

plugins/modules/inventory_workflow_manager.py

+                    self.log(self.msg, "ERROR")
+                    self.result["response"] = self.msg
+                    break
+


Added the Sleep log message.

…talyst-center-ansible-intg into inventory_workflow_manager

Inventory Workflow - Compute device related changes updated

06a7296

md-rafeek requested a review from madhansansel as a code owner August 18, 2025 20:50

Inventory Workflow - Compute device related changes updated

b299fbd

madhansansel reviewed Aug 25, 2025

View reviewed changes

Merge branch 'main' of https://github.com/cisco-en-programmability/ca…

417375c

…talyst-center-ansible-intg into inventory_workflow_manager

madhansansel reviewed Aug 25, 2025

View reviewed changes

md-rafeek added 3 commits August 26, 2025 08:56

Merge branch 'main' of https://github.com/cisco-en-programmability/ca…

2f54497

…talyst-center-ansible-intg into inventory_workflow_manager

Inventry Workflow - addressed code review command

5cf5a7e

Merge branch 'main' of https://github.com/cisco-en-programmability/ca…

c343092

…talyst-center-ansible-intg into inventory_workflow_manager

md-rafeek commented Aug 28, 2025

View reviewed changes

madhansansel reviewed Aug 28, 2025

View reviewed changes

md-rafeek added 2 commits August 29, 2025 15:52

Merge branch 'main' of https://github.com/cisco-en-programmability/ca…

546d717

…talyst-center-ansible-intg into inventory_workflow_manager

Inventory Workflow - addressed code review comments

5e67ea6

md-rafeek commented Aug 29, 2025

View reviewed changes

madhansansel reviewed Sep 1, 2025

View reviewed changes

Inventry Workflow - Added Sleep statement

70e8ffd

md-rafeek commented Sep 1, 2025

View reviewed changes

madhansansel reviewed Sep 2, 2025

View reviewed changes

Inventory Workflow - Added the Sleep log message

d40be19

md-rafeek commented Sep 2, 2025

View reviewed changes

Merge branch 'main' of https://github.com/cisco-en-programmability/ca…

8103495

…talyst-center-ansible-intg into inventory_workflow_manager

Inventory Workflow - Compute device related changes updated #293

Are you sure you want to change the base?

Inventory Workflow - Compute device related changes updated #293

Uh oh!

Conversation

md-rafeek commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of Change

Description

Checklist

Ansible Best Practices

Documentation

Screenshots (if applicable)

Notes to Reviewers

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

md-rafeek Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

md-rafeek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

md-rafeek Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

md-rafeek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

md-rafeek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

md-rafeek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

md-rafeek commented Aug 18, 2025 •

edited

Loading

md-rafeek Aug 27, 2025 •

edited

Loading

md-rafeek Aug 27, 2025 •

edited

Loading