[minor_change] add support for checkmode in aci_rest module #470

akinross · 2023-08-30T12:12:24Z

fixes #436

codecov · 2023-08-30T12:17:46Z

Codecov Report

Patch coverage: 98.37% and project coverage change: -0.01% ⚠️

Comparison is base (658ea51) 96.53% compared to head (372d654) 96.53%.
Report is 12 commits behind head on master.

❗ Current head 372d654 differs from pull request most recent head c47ff1b. Consider uploading reports for the commit c47ff1b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #470      +/-   ##
==========================================
- Coverage   96.53%   96.53%   -0.01%     
==========================================
  Files         181      183       +2     
  Lines        8291     8456     +165     
  Branches     1222     1259      +37     
==========================================
+ Hits         8004     8163     +159     
- Misses        217      222       +5     
- Partials       70       71       +1

Flag	Coverage Δ
integration	`94.54% <97.29%> (+0.01%)`	⬆️
sanity	`35.27% <17.29%> (-0.32%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
plugins/modules/aci_rest.py	`93.47% <95.65%> (+0.40%)`	⬆️
.../modules/aci_interface_policy_leaf_policy_group.py	`98.94% <97.14%> (-1.06%)`	⬇️
...dules/aci_interface_policy_leaf_fc_policy_group.py	`97.91% <97.91%> (ø)`
plugins/modules/aci_dhcp_relay.py	`100.00% <100.00%> (ø)`
plugins/modules/aci_dhcp_relay_provider.py	`100.00% <100.00%> (+2.12%)`	⬆️
...modules/aci_interface_policy_spine_policy_group.py	`100.00% <100.00%> (ø)`
plugins/modules/aci_l3out_extepg.py	`100.00% <100.00%> (ø)`

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sajagana

LGTM!

But could not test it completely because of the <urlopen error timed out>

shrsr · 2023-08-31T16:51:41Z

plugins/module_utils/aci.py

@@ -1446,6 +1446,9 @@ def exit_json(self, filter_existing=None, **kwargs):
                self.result["sent"] = self.config
                self.result["proposed"] = self.proposed

+        elif self.__class__.__name__ == "ACIRESTModule" and self.method in ["POST", "DELETE"]:


Why not do this->
elif "state" not in self.params?

Is possible but this assures it is only executed for ACIRESTModule class ( which is only used in aci_rest module). In case any other module has state not in self.params it would still execute which I want to avoid.

plugins/module_utils/aci.py

plugins/modules/aci_rest.py

anvitha-jain

LGTM

gmicol

LGTM

…esult for non checkmode operation

shrsr

LGTM

shrsr

LGTM

gmicol

can you check over the sanity test again? it seems like they were cancelled for some reason.

akinross · 2023-09-08T17:53:46Z

@gmicol when I run local I see only 2 pylint issues:

ERROR: Found 2 pylint issue(s) which need to be resolved:
ERROR: tests/unit/compat/builtins.py:30:4: unused-import: Unused import builtin
ERROR: tests/unit/module_utils/test_aci.py:44:8: unused-import: Unused cobra imported from xmljson

These are not related to the PR of mine. Also I do not see a reason for the cancel so not sure why this occurs

sajagana

LGTM!

… to the aci_rest module

shrsr

LGTM

sajagana

LGTM!

sajagana

LGTM!

shrsr

LGTM

lhercot

LGTM

plugins/module_utils/aci.py

[minor_change] add support for checkmode in aci_rest module

edb07d6

akinross requested review from lhercot, shrsr, anvitha-jain, sajagana and gmicol August 30, 2023 12:12

sajagana previously approved these changes Aug 31, 2023

View reviewed changes

shrsr requested changes Aug 31, 2023

View reviewed changes

akinross dismissed sajagana’s stale review via a786ff6 September 1, 2023 10:00

akinross requested a review from shrsr September 1, 2023 10:00

[ignore] set method for file output write in checkmode for aci_rest

d328643

akinross force-pushed the 436_check_mode_aci_rest branch from a786ff6 to d328643 Compare September 1, 2023 10:03

akinross requested a review from sajagana September 1, 2023 10:04

anvitha-jain previously approved these changes Sep 7, 2023

View reviewed changes

gmicol previously approved these changes Sep 8, 2023

View reviewed changes

[ignore] generic output write function in aci.py and propose add to r…

fb6cc63

…esult for non checkmode operation

akinross dismissed stale reviews from gmicol and anvitha-jain via fb6cc63 September 8, 2023 14:40

akinross requested review from gmicol and anvitha-jain September 8, 2023 14:45

shrsr previously approved these changes Sep 8, 2023

View reviewed changes

[ignore] black formatting

a29500a

akinross dismissed shrsr’s stale review via a29500a September 8, 2023 15:47

akinross requested a review from shrsr September 8, 2023 15:48

shrsr previously approved these changes Sep 8, 2023

View reviewed changes

anvitha-jain previously approved these changes Sep 8, 2023

View reviewed changes

gmicol requested changes Sep 8, 2023

View reviewed changes

akinross requested a review from gmicol September 8, 2023 17:55

gmicol previously approved these changes Sep 12, 2023

View reviewed changes

sajagana previously approved these changes Sep 14, 2023

View reviewed changes

[ignore] move proposed and output_path logic for aci_rest from aci.py…

423781f

… to the aci_rest module

akinross dismissed stale reviews from sajagana, gmicol, anvitha-jain, and shrsr via 423781f September 20, 2023 07:41

akinross requested review from shrsr, anvitha-jain, gmicol and sajagana September 20, 2023 07:43

shrsr previously approved these changes Sep 20, 2023

View reviewed changes

sajagana previously approved these changes Sep 20, 2023

View reviewed changes

[ignore] fix json dump consistency for aci_rest

c47ff1b

akinross dismissed stale reviews from sajagana and shrsr via c47ff1b September 20, 2023 16:44

anvitha-jain approved these changes Sep 20, 2023

View reviewed changes

akinross requested review from shrsr and sajagana September 22, 2023 12:53

sajagana approved these changes Sep 22, 2023

View reviewed changes

shrsr approved these changes Sep 22, 2023

View reviewed changes

gmicol approved these changes Sep 27, 2023

View reviewed changes

lhercot approved these changes Sep 28, 2023

View reviewed changes

plugins/module_utils/aci.py Outdated Show resolved Hide resolved

lhercot merged commit f9b55a4 into CiscoDevNet:master Sep 28, 2023
19 of 21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[minor_change] add support for checkmode in aci_rest module #470

[minor_change] add support for checkmode in aci_rest module #470

akinross commented Aug 30, 2023

codecov bot commented Aug 30, 2023 •

edited

Loading

sajagana left a comment

shrsr Aug 31, 2023

akinross Sep 1, 2023

anvitha-jain left a comment

gmicol left a comment

shrsr left a comment

shrsr left a comment

gmicol left a comment

akinross commented Sep 8, 2023 •

edited

Loading

sajagana left a comment

shrsr left a comment

sajagana left a comment

sajagana left a comment

shrsr left a comment

lhercot left a comment

[minor_change] add support for checkmode in aci_rest module #470

[minor_change] add support for checkmode in aci_rest module #470

Conversation

akinross commented Aug 30, 2023

codecov bot commented Aug 30, 2023 • edited Loading

Codecov Report

sajagana left a comment

Choose a reason for hiding this comment

shrsr Aug 31, 2023

Choose a reason for hiding this comment

akinross Sep 1, 2023

Choose a reason for hiding this comment

anvitha-jain left a comment

Choose a reason for hiding this comment

gmicol left a comment

Choose a reason for hiding this comment

shrsr left a comment

Choose a reason for hiding this comment

shrsr left a comment

Choose a reason for hiding this comment

gmicol left a comment

Choose a reason for hiding this comment

akinross commented Sep 8, 2023 • edited Loading

sajagana left a comment

Choose a reason for hiding this comment

shrsr left a comment

Choose a reason for hiding this comment

sajagana left a comment

Choose a reason for hiding this comment

sajagana left a comment

Choose a reason for hiding this comment

shrsr left a comment

Choose a reason for hiding this comment

lhercot left a comment

Choose a reason for hiding this comment

codecov bot commented Aug 30, 2023 •

edited

Loading

akinross commented Sep 8, 2023 •

edited

Loading