feat(pkg-mgr): Support dependency version upgrades #6017
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
11 tasks
ezgidemirel
force-pushed
the
dependency-version-update
branch
3 times, most recently
from
a329967 to
1633d9d
Compare
|
@ezgidemirel noticed that some of the review comments I left on #5991 still apply here. Could you please do a quick pass and resolve them here and/or hit resolve there if they are no longer relevant |
turkenh
reviewed
Oct 19, 2024
Comment on lines
84
to
93
| reasonErrBuildDAG event.Reason = "BuildDAGError" | ||
| reasonCyclicDependency event.Reason = "CyclicDependency" | ||
| reasonInvalidDependency event.Reason = "InvalidDependency" | ||
| reasonNoValidVersion event.Reason = "NoValidVersion" | ||
| reasonErrCreate event.Reason = "CreateError" | ||
| reasonErrUpdate event.Reason = "UpdateError" |
There was a problem hiding this comment.
Could be a follow up, but, I am starting to think to have a condition on the lock objects instead or in addition 🤔
Signed-off-by: ezgidemirel <ezgidemirel91@gmail.com>
ezgidemirel
force-pushed
the
dependency-version-update
branch
3 times, most recently
from
e6d8aeb to
1e8475f
Compare
Signed-off-by: ezgidemirel <ezgidemirel91@gmail.com>
ezgidemirel
force-pushed
the
dependency-version-update
branch
from
1e8475f to
01fb4d1
Compare
turkenh
force-pushed
the
dependency-version-update
branch
from
36bc5b5 to
428ac7f
Compare
Signed-off-by: Hasan Turken <turkenh@gmail.com>
turkenh
force-pushed
the
dependency-version-update
branch
from
428ac7f to
44dba2c
Compare
turkenh
changed the title
Signed-off-by: ezgidemirel <ezgidemirel91@gmail.com>
turkenh
approved these changes
Oct 21, 2024
There was a problem hiding this comment.
Great work implementing a long-awaited functionality, thanks @ezgidemirel 🙌
The PR looks good to me with the following open points which we could tackled as quick follow-ups:
- Use e2e artifacts from
xpkg.upbound.io/crossplane/...and include metadata to generate those packages just like other test cases. - Extend e2e coverage including ensuring we prevent downgrades, finding the lowest possible version if multiple packages rely on the same package with different constraints etc.
- Error reporting and propagation. We are introducing lock error reporting with events in this PR and I am not sure if it would be better if we introduced a condition. Then, we could propagate that condition to packages or packagerevisions so that one could see what is wrong in case of a package fails with dependency resolution.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of your changes
How Package Manager Works
How Lock Reconciler Works Before & After This Change
I used following packages to test different scenarios. You can find the package dependencies and tag conventions below:
gt: greater thandod: depends on digestSuccess installations:
Failure insallations:
events on lock obj:
events on lock obj:
Success upgrades:
xpkg.upbound.io/ezgiorg/ezgi-configuration-nopversion update fromv0.1.0tov0.1.1causes dependency version updateFixes #4511
I have:
earthly +reviewableto ensure this PR is ready for review.Addedbackport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.