MGMT-12996: enhancement for dynamic OCP fetching #4893
Conversation
Suggesting how assisted-service and assisted-installer UI can be adjusted to deliver all versions in the upgrade-channel API without the need of specified configuration.
openshift-ci
bot
added
the
size/L
openshift-ci
bot
added
the
approved
|
/hold for getting all the feedback |
openshift-ci
bot
requested review from
avishayt,
carbonin,
danielerez,
filanov and
gamli75
openshift-ci
bot
added
the
do-not-merge/hold
osherdp
force-pushed
the
enhancement/dynamic-fetching-of-openshif-versions
branch
from
e43d718
to
4f14d9e
Compare
Mainly around those points: * Mentioning more explicitly how assisted-image-service cops with the changes. * Provide more APIs for better transparency and for the UI to be able to only fetch the relevant versions. * Wording and repharsing changes.
osherdp
force-pushed
the
enhancement/dynamic-fetching-of-openshif-versions
branch
from
4f14d9e
to
e20563a
Compare
|
Looks good to me. Any other feedback or are we good to merge? |
|
@gamli75 @filanov @danielerez @avishayt anything else here? |
Looks good to me. |
|
Looks good to me. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gamli75, osherdp 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 |
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@osherdp: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
| description: It true, returns only the latest version for each minor. | ||
| type: boolean | ||
| allowEmptyValue: true | ||
| default: false |
There was a problem hiding this comment.
@avishayt @danielerez do you think it should be true by default?
otherwise it returns a big list that might be a bit weird for some API consumers
There was a problem hiding this comment.
Yeah, changing the current behaviour of the API might be confusing for existing users.
What about introducing instead a property for getting all versions? Something like 'include_all_minor' boolean.
From the UI perspective, it seems that only two variants will be used: the old behaviour (only latest) and version_pattern. So getting all minor versions seems like a rare option that deserves a separate special param.
There was a problem hiding this comment.
include_all_minor is the same as having the proposed only_latest, no?
There was a problem hiding this comment.
include_all_minoris the same as having the proposedonly_latest, no?
Yeah, just the opposite :) Not crucial, just sounds more straightforward than 'only_latest=false'
There was a problem hiding this comment.
hmm, by the way minor is the y part so do you mean include_all_patch_versions?
There was a problem hiding this comment.
hmm, by the way minor is the y part so do you mean
include_all_patch_versions?
right, tried to shorten 'include all for each minor', naming is hard:) maybe can be just 'all_versions' and explain in the description what is returned exactly.
There was a problem hiding this comment.
Sounds good
Thanks!
* MGMT-12996: enhancement for dynamic OCP fetching Suggesting how assisted-service and assisted-installer UI can be adjusted to deliver all versions in the upgrade-channel API without the need of specified configuration. * Apply suggestions from reviews Mainly around those points: * Mentioning more explicitly how assisted-image-service cops with the changes. * Provide more APIs for better transparency and for the UI to be able to only fetch the relevant versions. * Wording and repharsing changes. * change to in-memory solution * rename DYNAMIC_OPENSHIFT_RELEASES_CONFIG->RELEASE_SOURCES and Complete->Candidate * mention the remaining minor issues in 'risks and mitigations' section
Suggesting how assisted-service and assisted-installer UI can be adjusted to deliver all versions in the upgrade-channel API without the need of specified configuration.
List all the issues related to this PR
What environments does this code impact?
How was this code tested?
Checklist
docs, README, etc)Reviewers Checklist