Skip to content

Added NetFlow Exporter and Monitor tenant policy modules (DCNE-368 DCNE-369)#733

Open
sajagana wants to merge 5 commits intoCiscoDevNet:masterfrom
sajagana:652_netflow_monitor
Open

Added NetFlow Exporter and Monitor tenant policy modules (DCNE-368 DCNE-369)#733
sajagana wants to merge 5 commits intoCiscoDevNet:masterfrom
sajagana:652_netflow_monitor

Conversation

@sajagana
Copy link
Collaborator

No description provided.

@sajagana sajagana force-pushed the 652_netflow_monitor branch 3 times, most recently from 87578b5 to eb1e410 Compare February 10, 2026 06:39
@sajagana sajagana marked this pull request as ready for review February 10, 2026 06:47
@sajagana sajagana changed the title Added NetFlow Exporter and Monitor tenant policy modules Added NetFlow Exporter and Monitor tenant policy modules (DCNE-368, DCNE-369) Feb 10, 2026
@github-actions github-actions bot changed the title Added NetFlow Exporter and Monitor tenant policy modules (DCNE-368, DCNE-369) Added NetFlow Exporter and Monitor tenant policy modules (, ) (DCNE-368 DCNE-369) Feb 10, 2026
required_if=[
["state", "absent", ["name", "uuid"], True],
["state", "present", ["name", "uuid"], True],
["state", "present", ["netflow_exporters"]],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we sure about this required_if condition?
At least one NetFlow exporter object must be attached to a NetFlow monitor during creation.
But our condition, here, extends that situation to even updates: every time we want to update NetFlow Monitor we need to provide the NetFlow exporters even though we don't want to update it (and it's not an identifier either so it's not really needed).

We should probably fix this.

Also the API returns a proper error when a NetFlow Monitor is created without at least one Exporter so we should probably rely on the API behavior for this. (Have you checked what happened when you update an existing correct Monitor object and you remove all Exporters from it? What is the API behavior here?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for the feedback! Updated module and test file.

@sajagana sajagana requested a review from gmicol February 18, 2026 11:35
@sajagana sajagana force-pushed the 652_netflow_monitor branch from 2609955 to 4105bf8 Compare February 23, 2026 15:15
@sajagana sajagana requested a review from akinross February 23, 2026 15:29
@sajagana sajagana changed the title Added NetFlow Exporter and Monitor tenant policy modules (, ) (DCNE-368 DCNE-369) Added NetFlow Exporter and Monitor tenant policy modules (DCNE-368 DCNE-369) Feb 23, 2026
akinross
akinross previously approved these changes Feb 25, 2026
Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

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

LGTM

description:
- The NetFlow Record reference details for the NetFlow Monitor.
- Providing an empty dictionary O(netflow_record={}) will remove NetFlow Record from the NetFlow Monitor.
- Defaults to an empty string when unset during creation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this intended to say empty dictionary or empty string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed the unwanted description.

samiib
samiib previously approved these changes Mar 4, 2026
Copy link
Collaborator

@samiib samiib left a comment

Choose a reason for hiding this comment

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

LGTM

akinross
akinross previously approved these changes Mar 5, 2026
Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

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

LGTM

gmicol
gmicol previously approved these changes Mar 10, 2026
Copy link
Collaborator

@gmicol gmicol left a comment

Choose a reason for hiding this comment

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

LGTM

@sajagana sajagana dismissed stale reviews from gmicol, akinross, and samiib via 7e10953 March 17, 2026 14:50
@sajagana sajagana force-pushed the 652_netflow_monitor branch from 2ee550b to 7e10953 Compare March 17, 2026 14:50
@sajagana sajagana requested review from akinross, gmicol and samiib March 17, 2026 14:52
Copy link
Collaborator

@samiib samiib left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@gmicol gmicol left a comment

Choose a reason for hiding this comment

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

LGTM

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

Labels

None yet

Projects

None yet

4 participants