-
Notifications
You must be signed in to change notification settings - Fork 6
Fix upstream mariadb upgrade #39
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
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but check for comment messages please.
| url_parts[1] = url_parts[1].replace('$releasever', str(target_major)) | ||
| return "yum".join(url_parts) | ||
| else: | ||
| # TODO: fix in https://cloudlinux.atlassian.net/browse/CLOS-3490 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is created in scope of CLOS-3490 as far as i can see. Old code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I initially thought the else branch needed adjustment when I wrote that comment, but it turned out all the URLs contain 'yum' - at least all the ones I could find.
Thanks, removed that comment.
| # Replace the first occurrence of source_major with target_major after 'yum' | ||
| url_parts = mariadb_url.split("yum", 1) | ||
| if len(url_parts) == 2: | ||
| # Replace major version in "/centos/7/" and /yum/12.0/almalinux9-amd64/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/yum/12.0/almalinux9-amd64
Nitpick: yum is a delimiter. It's not included.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed it from the URL to avoid confusion.
920e0ca to
4e91b1f
Compare
This change fixes upstream mariadb upgrade procedure by better baseurl links handling.
Also, upgrades from mariadb 10.3 and 10.4 on CloudLinux 8 are blocked because of the bug in spec which hangs upgrade process .