Skip to content

[minor_change] Added Suppress Previous option to reduce the number of API calls. #636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Apr 17, 2024
Merged

[minor_change] Added Suppress Previous option to reduce the number of API calls. #636

merged 7 commits into from
Apr 17, 2024

Conversation

@samiib
Copy link
Collaborator

@samiib samiib commented Apr 2, 2024

  • If true, a look back GET will not be sent before a POST update to APIC.
  • The previous state of the object will not be checked and the POST update will contain all configred properties.
  • Additionally, added a fix to reduce GET calls when an object is unchanged.

@samiib samiib force-pushed the 609_supress_look_back branch from 2e92b96 to 3f6c022 Compare April 2, 2024 11:38
@codecov
Copy link

codecov bot commented Apr 2, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 6.25000% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 35.68%. Comparing base (faff94c) to head (a68a92c).
Report is 1 commits behind head on master.

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

Files Patch % Lines
plugins/module_utils/aci.py 6.25% 15 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #636      +/-   ##
==========================================
- Coverage   35.70%   35.68%   -0.02%     
==========================================
  Files         265      265              
  Lines       12133    12141       +8     
  Branches     1835     1837       +2     
==========================================
+ Hits         4332     4333       +1     
- Misses       7801     7808       +7
Flag Coverage Δ
sanity 35.68% <6.25%> (-0.02%) ⬇️

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.

akinross
akinross requested changes Apr 2, 2024
View reviewed changes
@samiib samiib self-assigned this Apr 2, 2024
shrsr
shrsr requested changes Apr 3, 2024
View reviewed changes
@shrsr
Copy link
Collaborator

shrsr commented Apr 3, 2024 •
edited
Loading

Is this PR part of the issue #609?

@shrsr shrsr linked an issue Apr 3, 2024 that may be closed by this pull request
Performance evaluation and recommendation for improvement #609
Closed
@shrsr shrsr removed a link to an issue Apr 3, 2024
Performance evaluation and recommendation for improvement #609
Closed
@samiib
Copy link
Collaborator Author

samiib commented Apr 3, 2024 •
edited
Loading

Is this PR part of the issue #609?

Yes, thanks for linking sorry I may have forgotten to link it this time. I still got to do another PR so can't link just yet.

@samiib samiib linked an issue Apr 3, 2024 that may be closed by this pull request
Performance evaluation and recommendation for improvement #609
Closed
@samiib samiib removed a link to an issue Apr 3, 2024
Performance evaluation and recommendation for improvement #609
Closed
@samiib samiib changed the title [minor_change] Added Suppress Look Back option to reduce the number of API calls. [minor_change] Added Suppress Previous option to reduce the number of API calls. Apr 4, 2024
@samiib samiib requested review from akinross and shrsr April 4, 2024 11:38
akinross
akinross previously approved these changes Apr 9, 2024
View reviewed changes
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 requested changes Apr 9, 2024
View reviewed changes
@samiib samiib force-pushed the 609_supress_look_back branch from f9462e3 to 9fa2464 Compare April 10, 2024 05:30
@samiib samiib requested review from akinross and gmicol April 10, 2024 06:20
samiib
samiib commented Apr 10, 2024
View reviewed changes
akinross
akinross previously approved these changes Apr 10, 2024
View reviewed changes
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

samiib
samiib commented Apr 10, 2024
View reviewed changes
@samiib samiib linked an issue Apr 11, 2024 that may be closed by this pull request
Performance evaluation and recommendation for improvement #609
Closed
@samiib samiib removed a link to an issue Apr 11, 2024
Performance evaluation and recommendation for improvement #609
Closed
samiib added 4 commits April 11, 2024 17:27
@samiib
[minor_change] Added suppress-look-back flag option to reduce the num…
5701931 
…ber of API calls.

- If true, a look back GET will not be sent before a POST update to APIC.
- The previous state of the object will not be checked and the POST update will contain all configred properties.
- Additionally, added a fix to reduce GET calls when an object is unchanged.
@samiib
[ignore] Added suppress-look-back flag option to reduce the number of…
faeca56 
… API calls.

- Added integration tests for suppress look back and suppress verify with suppress look back.
- Fixed some issues with suppress verify when proposed is empty.
- Fixed an issue on when using no look back and absent state.
@samiib
[ignore] Rename suppress_look_back to suppress_previous
6bd98ca
@samiib
[ignore] Added docstring to __get_existing_validation
90d26df
@samiib samiib force-pushed the 609_supress_look_back branch from 9fa2464 to 90d26df Compare April 11, 2024 07:28
gmicol
gmicol previously approved these changes Apr 11, 2024
View reviewed changes
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

@samiib samiib dismissed stale reviews from gmicol and akinross via 8acf8a0 April 11, 2024 21:19
@samiib samiib requested review from akinross and gmicol April 11, 2024 21:24
akinross
akinross previously approved these changes Apr 12, 2024
View reviewed changes
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

shrsr
shrsr requested changes Apr 12, 2024
View reviewed changes
@samiib samiib requested review from akinross and shrsr April 15, 2024 06:06
akinross
akinross requested changes Apr 15, 2024
View reviewed changes
@samiib samiib requested a review from akinross April 15, 2024 08:23
akinross
akinross approved these changes Apr 15, 2024
View reviewed changes
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

shrsr
shrsr approved these changes Apr 15, 2024
View reviewed changes
Copy link
Collaborator

@shrsr shrsr left a comment

Choose a reason for hiding this comment

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

LGTM

anvitha-jain
anvitha-jain approved these changes Apr 15, 2024
View reviewed changes
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.

LGTM

sajagana
sajagana approved these changes Apr 16, 2024
View reviewed changes
Copy link
Collaborator

@sajagana sajagana 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 approved these changes Apr 16, 2024
View reviewed changes
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

lhercot
lhercot approved these changes Apr 17, 2024
View reviewed changes
Copy link
Member

@lhercot lhercot left a comment

Choose a reason for hiding this comment

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

LGTM

@lhercot lhercot merged commit 41f074d into CiscoDevNet:master Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lhercot lhercot lhercot approved these changes

@shrsr shrsr shrsr approved these changes

@akinross akinross akinross approved these changes

@anvitha-jain anvitha-jain anvitha-jain approved these changes

@sajagana sajagana sajagana approved these changes

@gmicol gmicol gmicol approved these changes

Assignees

@samiib samiib

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@samiib @shrsr @lhercot @akinross @anvitha-jain @sajagana @gmicol