convert dot-less CentOS versions to X.999 #1139
Conversation
Thank you for contributing to the Leapp project!Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergeable.
Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build. If you need a different version of leapp from PR#42, use It is possible to schedule specific on-demand tests as well. Currently 2 test sets are supported,
[Deprecated] To launch on-demand regression testing public members of oamg organization can leave the following comment:
Please open ticket in case you experience technical problem with the CI. (RH internal only) Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please contact leapp-infra. |
| release_id = data.get('ID', '').strip('"') | ||
| version_id = data.get('VERSION_ID', '').strip('"') | ||
| if release_id == 'centos' and '.' not in version_id: | ||
| version_id = "{}.999".format(version_id) |
There was a problem hiding this comment.
I am not too happy about 999 magic constant tbh :(
Is there a universe where version_id doesn't have 999 and leapp is still happy?
There was a problem hiding this comment.
Honestly, I do not think this is the right solution for this. This should not be resolved by magical/crafted values that have nothing to do with the real system. This particular problem will need to be refink, together with the target os version - where this magic even cannot be used as it has effect on baseurls of target repositories if $releasever is present.
Please note that we do not have a capacity right now to think about the right solution, so it's possible we will need to postpone this for a time. But when we have a time, we could invite you to our team tech. mtg to discuss the solution if you are interested. Once we have an agreement how the solution should look like, we can specify in github issues to set properly the expectation about the solution.
|
Yeah, I agree, this is not a nice solution (but it works 🙃) I'll happily join the mtg to discuss this! |
|
Just in case here are the commits we made in ELevate project to fix this CentOS version issue: But this PR looks cleaner. |
CentOS Stream doesn't do dot releases, so it always has the major version in the version_id field of os-release. Our code, in multiple places, expects versions in the form X.Y and doesn't cope well when given only X. The value "999" is taken arbitrarily, with the sole assumption that it will be always higher than any RHEL release, as CentOS Stream is ahead of RHEL. See https://issues.redhat.com/browse/CS-1757 for further discussion and alternative numbering options.
evgeni
force-pushed
the
centos-stream
branch
from
1682dd9
to
c272dc7
Compare
CentOS Stream doesn't do dot releases, so it always has the major version in the version_id field of os-release.
Our code, in multiple places, expects versions in the form X.Y and doesn't cope well when given only X.
The value "999" is taken arbitrarily, with the sole assumption that it will be always higher than any RHEL release, as CentOS Stream is ahead of RHEL.
See https://issues.redhat.com/browse/CS-1757 for further discussion and alternative numbering options.