o/registrystate: manage accesses to registries #14544
Conversation
MiguelPires
added
the
confdb
github-actions
bot
added
the
Needs Documentation -auto-
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14544 +/- ##
==========================================
+ Coverage 78.87% 78.91% +0.03%
==========================================
Files 1083 1083
Lines 146109 146480 +371
==========================================
+ Hits 115244 115590 +346
- Misses 23668 23686 +18
- Partials 7197 7204 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This change changes the registries accesses to use GetTransaction which determines which kind of access it's dealing with (e.g,. snap set, snapctl set, in or out of a hook, registry hook or non-registry, etc) and takes the appropriate action. This omits the concurrency checks and I've also opted to removed the bits allowing reading concurrently if the registry being read is different than the one with a transaction ongoing (this will require changes anyway since we'll add a load-registry change in the future). At the moment, the user API endpoint isn't connected to this yet since that will require more changes to it. Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
MiguelPires
force-pushed
the
registry-get-tx
branch
from
e44d42a
to
00b806c
Compare
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
| return fmt.Errorf("cannot modify registry in %q hook", ctx.HookName()) | ||
| } | ||
|
|
||
| regCtx := registrystate.NewContext(ctx) |
There was a problem hiding this comment.
we cannot use onDone on regCtx here, so not sure why we don't pass ctx and GetTransaction makes then its Context?
There was a problem hiding this comment.
The registrystate.Context is a way to defer the creation of the change or updating of state until the right time. For a registry hook, that might be when it ends. The API will not block but it will trigger it and then get the change (it'll need minor changes for that) and include it in the http response
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
Make reads through snapctl go through a different flow than the writes. This will change in the future as the reads will also trigger a change to load data but for now this makes access logic easier to reason about and it's also slightly more correct. Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
|
@andrewphelpsj can you have a look at the last commit c268103 please? It makes reads go through a simpler flow which is more correct than having it just reuse the write side access function and is also easier to reason about. I was going to do this separately but, for expediency reasons, I'm including it here. thanks |
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
|
The failure is an unrelated timeout in the document-interfaces-url test on 24.04 |
This change changes the registries accesses to use GetTransaction that determines which kind of access it's dealing with (e.g,. snap set, snapctl set, in or out of a hook, registry hook or non-registry, etc). This omits the concurrency checks and I've also opted to removed the bits allowing reading concurrently if the registry being read is different than the one with a transaction ongoing (this will require changes anyway since we'll add a load-registry change in the future). At the moment, the user API endpoint isn't connected to this yet since that will require more changes to it and this is already large enough.