pkg/sqlite: use consistent starting depth when building channel_entry table #868
Conversation
… table When building the channel_entry table, always using a starting depth of 0. This is important because the ListBundles query makes an assumption that all packages in the database will have the same minimum depth. Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
openshift-ci
bot
added
the
approved
Codecov Report
@@ Coverage Diff @@
## master #868 +/- ##
==========================================
+ Coverage 51.51% 52.07% +0.56%
==========================================
Files 103 103
Lines 9062 9063 +1
==========================================
+ Hits 4668 4720 +52
+ Misses 3515 3438 -77
- Partials 879 905 +26
Continue to review full report at Codecov.
|
| for _, b := range bundles { | ||
| graph, err := graphLoader.Generate(b.Package) | ||
| require.Conditionf(t, func() bool { | ||
| return err == nil || errors.Is(err, registry.ErrPackageNotInDatabase) | ||
| }, "got unexpected error: %v", err) | ||
| bundleLoader := registry.BundleGraphLoader{} | ||
| updatedGraph, err := bundleLoader.AddBundleToGraph(b, graph, ®istry.AnnotationsFile{Annotations: *b.Annotations}, false) | ||
| require.NoError(t, err) | ||
| err = store.AddBundleSemver(updatedGraph, b) | ||
| require.NoError(t, err) | ||
| } |
There was a problem hiding this comment.
I don't love that all of this code is in the test. It seems like this should be better encapsulated in non-test code. I suppose I could re-write the test using the DirectoryPopulator abstraction, but that would require using the imageDirMap and putting a bunch of bundle directories to disk during the test.
We could also look at making this something that could be more easily called in a test.
I'm just not sure it's worth it given all of this code is deprecated.
dinhxuanvu
left a comment
dinhxuanvu
left a comment
There was a problem hiding this comment.
Looking good. A few comments.
pkg/sqlite/load.go
Outdated
| // we got to the end of the channel graph | ||
| if nextNode.IsEmpty() { | ||
| if len(channel.Nodes) != depth { | ||
| if len(channel.Nodes)+startDepth-1 != depth { |
There was a problem hiding this comment.
Would be nice to have a comment explaining this addition.
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
dinhxuanvu
left a comment
dinhxuanvu
left a comment
There was a problem hiding this comment.
Satisfied with the latest changes.
/lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dinhxuanvu, joelanford, perdasilva The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 participants
Description of the change:
When building the channel_entry table, always using a starting depth of 0.
Motivation for the change:
This is important because the ListBundles query makes an assumption that all packages in the database will have the same minimum depth.
Closes #867
Signed-off-by: Joe Lanford <joe.lanford@gmail.com
Reviewer Checklist
/docs