[minor change] Add support for annotation in aci_rest module (#437) #497

samiib · 2023-10-19T10:48:21Z

fixes #437

codecov · 2023-10-19T22:00:23Z

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (324327f) 35.32% compared to head (c6a7c6c) 96.25%.

Files	Patch %	Lines
plugins/modules/aci_rest.py	93.75%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #497       +/-   ##
===========================================
+ Coverage   35.32%   96.25%   +60.92%     
===========================================
  Files         230      231        +1     
  Lines       10610    10640       +30     
  Branches     1591     1601       +10     
===========================================
+ Hits         3748    10241     +6493     
+ Misses       6862      306     -6556     
- Partials        0       93       +93

Flag	Coverage Δ
integration	`94.71% <93.93%> (?)`
sanity	`35.27% <18.18%> (-0.06%)`	⬇️

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

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

plugins/modules/aci_rest.py

akinross

LGTM

sajagana

cm_add_tenant.proposed.fvTenant.attributes.annotation == "orchestrator:ansible" check is failing.

samiib · 2023-12-04T03:08:55Z

cm_add_tenant.proposed.fvTenant.attributes.annotation == "orchestrator:ansible" check is failing.

@sajagana This check in the xml_string tests should be fixed now.

akinross

LGTM

sajagana

Also update the tests/integration/targets/aci_rest/tasks/error_handling.yml task: - name: Verify error_on_input_validation

-    - "error_on_input_validation.msg == 'APIC Error 801: property descr of tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"
+    - "error_on_input_validation.msg == 'APIC Error 801: property descr of uni/tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"

tests/integration/targets/aci_rest/tasks/xml_file.yml

samiib · 2023-12-05T06:38:17Z

Also update the tests/integration/targets/aci_rest/tasks/error_handling.yml task: - name: Verify error_on_input_validation

-    - "error_on_input_validation.msg == 'APIC Error 801: property descr of tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"
+    - "error_on_input_validation.msg == 'APIC Error 801: property descr of uni/tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"

@sajagana, Not sure about this one because the test was passing when I ran it myself. Just wondering which APIC did you use to test on?

Co-authored-by: Sabari Jaganathan <[email protected]>

akinross · 2023-12-20T18:36:29Z

Also update the tests/integration/targets/aci_rest/tasks/error_handling.yml task: - name: Verify error_on_input_validation
-    - "error_on_input_validation.msg == 'APIC Error 801: property descr of tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"
+    - "error_on_input_validation.msg == 'APIC Error 801: property descr of uni/tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"
@sajagana, Not sure about this one because the test was passing when I ran it myself. Just wondering which APIC did you use to test on?

I'm seeing the same error now when running tests ( did not see runs before ). On november 28th the APICs where upgraded as I recall, perhaps the return message has changed?

…ci_rest_annotation

…sion updates.

samiib · 2023-12-21T00:17:40Z

Also update the tests/integration/targets/aci_rest/tasks/error_handling.yml task: - name: Verify error_on_input_validation
-    - "error_on_input_validation.msg == 'APIC Error 801: property descr of tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"
+    - "error_on_input_validation.msg == 'APIC Error 801: property descr of uni/tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"
@sajagana, Not sure about this one because the test was passing when I ran it myself. Just wondering which APIC did you use to test on?
I'm seeing the same error now when running tests ( did not see runs before ). On november 28th the APICs where upgraded as I recall, perhaps the return message has changed?

@sajagana & @akinross

I have updated my branch with the latest changes from master and have been able to reproduce the failed assertion.
It appears the error message has changed between different versions of APIC. Some versions include uni/ in the error and other versions don't.
To account for this difference I changed the assertion to use a simple regex so the test will pass for this message variation.

sajagana

LGTM!

akinross

LGTM

lhercot

LGTM

samiib force-pushed the aci_rest_annotation branch from 57f718c to 10a328d Compare October 19, 2023 21:43

[minor change] Add support for annotation in aci_rest module (CiscoDe…

eae0317

…vNet#437)

samiib force-pushed the aci_rest_annotation branch from 10a328d to eae0317 Compare October 19, 2023 21:55

samiib changed the title ~~[minor change] Add support for annotation in aci_rest module (#437)~~ WIP [minor change] Add support for annotation in aci_rest module (#437) Oct 19, 2023

[ignore] Handle classes that don't support annotation

b5986c0

samiib requested review from lhercot, akinross and sajagana October 20, 2023 09:57

samiib changed the title ~~WIP [minor change] Add support for annotation in aci_rest module (#437)~~ [minor change] Add support for annotation in aci_rest module (#437) Oct 20, 2023

samiib self-assigned this Oct 20, 2023

samiib added the enhancement New feature or request label Oct 20, 2023

samiib linked an issue Oct 20, 2023 that may be closed by this pull request

aci_rest should support annotation option #437

Closed

samiib requested review from shrsr, anvitha-jain and gmicol October 20, 2023 10:06

akinross requested changes Oct 24, 2023

View reviewed changes

plugins/modules/aci_rest.py Outdated Show resolved Hide resolved

plugins/modules/aci_rest.py Outdated Show resolved Hide resolved

plugins/modules/aci_rest.py Show resolved Hide resolved

[ignore] Added all unsupported annotation classes.

8c5085f

samiib force-pushed the aci_rest_annotation branch 3 times, most recently from 45f2313 to 5167abb Compare October 25, 2023 22:16

[ignore] Add integration testing for annotation on aci_rest module

0f29c6f

samiib force-pushed the aci_rest_annotation branch from 5167abb to 0f29c6f Compare October 27, 2023 01:46

samiib requested a review from akinross October 27, 2023 08:24

[ignore] aci_rest add annotation to children and fix some tests

37f922d

samiib force-pushed the aci_rest_annotation branch from d7c3ddf to 37f922d Compare October 31, 2023 02:36

akinross previously approved these changes Oct 31, 2023

View reviewed changes

sajagana requested changes Nov 7, 2023

View reviewed changes

[ignore] Fix aci_rest xml_string tests

50631e8

samiib dismissed akinross’s stale review via 50631e8 December 4, 2023 03:00

samiib requested review from sajagana and akinross December 4, 2023 03:09

akinross previously approved these changes Dec 4, 2023

View reviewed changes

sajagana requested changes Dec 4, 2023

View reviewed changes

tests/integration/targets/aci_rest/tasks/xml_file.yml Outdated Show resolved Hide resolved

samiib dismissed akinross’s stale review via 4afbe0f December 5, 2023 06:46

[ignore] fix xml_file tests

4afbe0f

Co-authored-by: Sabari Jaganathan <[email protected]>

akinross requested review from sajagana and akinross December 12, 2023 17:05

samiib added 2 commits December 21, 2023 09:16

Merge branch 'master' of https://github.com/samiib/ansible-aci into a…

fbd1eeb

…ci_rest_annotation

[Ignore] Fix aci_rest error_on_input_validation assert after APIC ver…

c6a7c6c

…sion updates.

sajagana approved these changes Dec 21, 2023

View reviewed changes

akinross approved these changes Dec 21, 2023

View reviewed changes

lhercot approved these changes Dec 21, 2023

View reviewed changes

lhercot merged commit 647dc30 into CiscoDevNet:master Dec 21, 2023
19 of 23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[minor change] Add support for annotation in aci_rest module (#437) #497

[minor change] Add support for annotation in aci_rest module (#437) #497

samiib commented Oct 19, 2023 •

edited

Loading

codecov bot commented Oct 19, 2023 •

edited

Loading

akinross left a comment

sajagana left a comment

samiib commented Dec 4, 2023

akinross left a comment

sajagana left a comment

samiib commented Dec 5, 2023

akinross commented Dec 20, 2023

samiib commented Dec 21, 2023 •

edited

Loading

sajagana left a comment

akinross left a comment

lhercot left a comment

[minor change] Add support for annotation in aci_rest module (#437) #497

[minor change] Add support for annotation in aci_rest module (#437) #497

Conversation

samiib commented Oct 19, 2023 • edited Loading

codecov bot commented Oct 19, 2023 • edited Loading

Codecov Report

akinross left a comment

Choose a reason for hiding this comment

sajagana left a comment

Choose a reason for hiding this comment

samiib commented Dec 4, 2023

akinross left a comment

Choose a reason for hiding this comment

sajagana left a comment

Choose a reason for hiding this comment

samiib commented Dec 5, 2023

akinross commented Dec 20, 2023

samiib commented Dec 21, 2023 • edited Loading

sajagana left a comment

Choose a reason for hiding this comment

akinross left a comment

Choose a reason for hiding this comment

lhercot left a comment

Choose a reason for hiding this comment

samiib commented Oct 19, 2023 •

edited

Loading

codecov bot commented Oct 19, 2023 •

edited

Loading

samiib commented Dec 21, 2023 •

edited

Loading