Skip to content
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

status: refactor update contract info to be a background job (SC-1411) #2409

Merged
merged 5 commits into from Feb 16, 2023
Merged

status: refactor update contract info to be a background job (SC-1411) #2409

merged 5 commits into from Feb 16, 2023

Conversation

aciba90
Copy link
Collaborator

@aciba90 aciba90 commented Jan 31, 2023 •
edited

Proposed Commit Message

status: refactor update contract info to be a background job

At the moment, the backend service call to check if the
contract info has to be updated can be slow under load,
and this was executed during `pro status`, blocking it
during some seconds.

Move the update contract info check as a background job
to improve user experience.

Test Steps

Checklist

  • I have updated or added any unit tests accordingly
  • I have updated or added any integration tests accordingly
  • I have updated or added any documentation accordingly

Does this PR require extra reviews?

  • Yes
  • No

@aciba90 aciba90 changed the title status: refactor update contract info to be a background job status: refactor update contract info to be a background job (SC-1411) Feb 7, 2023
@aciba90
Copy link
Collaborator Author

aciba90 commented Feb 7, 2023

I have cherry-picked 72bdfe2 to fix a motd test and there is another unrelated test failing:

features/api_security.feature:69 Call package manifest endpoint for machine

@aciba90 aciba90 marked this pull request as ready for review February 7, 2023 09:54
orndorffgrant
orndorffgrant reviewed Feb 8, 2023
View reviewed changes
Copy link
Collaborator

@orndorffgrant orndorffgrant left a comment

Choose a reason for hiding this comment

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

Looking good! Some small suggestions

features/attach_validtoken.feature Outdated Show resolved Hide resolved
lib/timer.py Outdated Show resolved Hide resolved
uaclient/jobs/update_contract_info.py Outdated Show resolved Hide resolved
@aciba90
Copy link
Collaborator Author

aciba90 commented Feb 9, 2023

Thanks for the feedback, @orndorffgrant, I have applied your comments/suggestions.

renanrodrigo
renanrodrigo approved these changes Feb 10, 2023
View reviewed changes
Copy link
Member

@renanrodrigo renanrodrigo left a comment

Choose a reason for hiding this comment

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

LGTM - couple small comments

uaclient/jobs/tests/test_update_contract_info.py Outdated Show resolved Hide resolved
uaclient/jobs/tests/test_update_contract_info.py Show resolved Hide resolved
aciba90 and others added 5 commits February 13, 2023 13:29
@aciba90
status: refactor update contract info to be a background job
b9cfc1e
@aciba90
fix timer and integration test
3e763a7
@lucasmoura @aciba90
tests: install ansible package for esm messages
028bb10 
For bionic, we now need to install a new package to
trigger esm-apps messages. The same package is also used
to correctly populate the security-status content on Bionic
@aciba90
PR comments
4600f54
@aciba90
test: PR comments
743a5b4
@orndorffgrant orndorffgrant merged commit 3e2da34 into main Feb 16, 2023
@orndorffgrant orndorffgrant deleted the contract_info_as_job branch February 16, 2023 15:25
@orndorffgrant orndorffgrant mentioned this pull request Feb 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orndorffgrant orndorffgrant orndorffgrant approved these changes

@renanrodrigo renanrodrigo renanrodrigo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@aciba90 @orndorffgrant @renanrodrigo @lucasmoura