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.

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

o/confdbstate: add helper to schedule load confdb hook/tasks #15110

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

miguelpires
Copy link
Contributor

81070b9 changes the key used to link tasks to the transaction carrying task
c3aca30 is a small refactor to prevent duplicating some code in the new helper
5490fc1 adds support for the query-view and load-view hooks as well as a helper that schedules tasks appropriately

The loading change will reuse GetStoredTransaction to obtain the
transaction stored or linked by a task. However, the confdb loading
doesn't commit anything so it doesn't make sense to store the task ID
under "commit-task". This changes that to "tx-task".

Signed-off-by: Miguel Pires <[email protected]>
Refactors the accumulating of custodian snap names into the same
function that looks for relevant plugs so we don't need to duplicate it
in when we implement something similar for the loading confdb change.

Signed-off-by: Miguel Pires <[email protected]>
@miguelpires miguelpires added the confdb confdb work (previously called registries and before aspects) label Feb 20, 2025
@github-actions github-actions bot added the Needs Documentation -auto- Label automatically added which indicates the change needs documentation label Feb 20, 2025
Copy link

github-actions bot commented Feb 20, 2025

Tue Feb 25 12:12:44 UTC 2025
The following results are from: https://github.com/canonical/snapd/actions/runs/13517011391

Failures:

Executing:

  • google:ubuntu-20.04-64:tests/main/lxd:snapd_cgroup_neither

Copy link
Contributor

@ZeyadYasser ZeyadYasser left a comment

Choose a reason for hiding this comment

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

Looks good, small question

Copy link
Contributor

@ZeyadYasser ZeyadYasser 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

codecov bot commented Feb 25, 2025

Codecov Report

Attention: Patch coverage is 89.55224% with 7 lines in your changes missing coverage. Please review.

Project coverage is 78.15%. Comparing base (a272aac) to head (524531b).
Report is 40 commits behind head on master.

Files with missing lines Patch % Lines
overlord/confdbstate/confdbstate.go 89.83% 5 Missing and 1 partial ⚠️
overlord/confdbstate/confdbmgr.go 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #15110      +/-   ##
==========================================
+ Coverage   78.07%   78.15%   +0.08%     
==========================================
  Files        1182     1177       -5     
  Lines      157743   157871     +128     
==========================================
+ Hits       123154   123387     +233     
+ Misses      26943    26830     -113     
- Partials     7646     7654       +8     
Flag Coverage Δ
unittests 78.15% <89.55%> (+0.08%) ⬆️

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confdb confdb work (previously called registries and before aspects) Needs Documentation -auto- Label automatically added which indicates the change needs documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants