-
Notifications
You must be signed in to change notification settings - Fork 254
WIP: FBC data model migrations #1679
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
base: master
Are you sure you want to change the base?
WIP: FBC data model migrations #1679
Conversation
Skipping CI for Draft Pull Request. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: joelanford 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 |
aa18b4b
to
aa53bbc
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1679 +/- ##
==========================================
- Coverage 55.17% 54.88% -0.30%
==========================================
Files 136 139 +3
Lines 15918 16161 +243
==========================================
+ Hits 8783 8870 +87
- Misses 5982 6118 +136
- Partials 1153 1173 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
aa53bbc
to
ba394f0
Compare
Signed-off-by: Joe Lanford <[email protected]>
Signed-off-by: Joe Lanford <[email protected]>
ba394f0
to
06f9b26
Compare
…ge blob Signed-off-by: Joe Lanford <[email protected]>
06f9b26
to
dd26966
Compare
Signed-off-by: Joe Lanford [email protected]<!--
Before making a PR, please read our contributing guidelines https://github.com/operator-framework/operator-lifecycle-manager/blob/master/CONTRIBUTING.md
Note: Make sure your branch is rebased to the latest upstream master.
-->
WIP: This is an implementation with no tests. Before merge, we need to add some tests to ensure migrations work as expected and GRPC compatibility is maintained.
Description of the change:
For all of these, the
opm serve
implementation is updated to correctly plumb the new fields into the GRPC API.Motivation for the change:
olm.package
property, where both the package name and version are required. The bundle schema already has the package name, and the bundle version is a key piece of information when dealing with bundles. Promoting the bundle version to a first class field and eliminating theolm.package
property from FBCs reduces duplication of package name and improves performance and ergonomics for clients that need to know the bundle version (which is virtually all clients).Open questions:
olm.package
's.icon
field when we migrate toolm.icon
? Clients that don't know about the newolm.icon
will think there is no icon. If we don't remove the.icon
field. The benefit of separating it out is completely eliminated and we might as well not separate it because then we'd just duplicate the icon unecessarily.olm.bundle
'solm.package
property when we promote the version to a first-class field? Same sort of problem as above, In this case, we could probably keep both the property and the version field, but then we would need extra validation to ensure they match and we'd be using slightly more space per bundle.Reviewer Checklist
/docs