Handle URL params for AptRepoFiles #3223
Conversation
|
cc: @sbernhard |
|
Hello 👋 I know it's still a draft, please take a look at the unit test failures (see the |
hstct
force-pushed
the
parse-url-params-for-repositories
branch
from
0043600 to
79af61d
Compare
|
Hi :) The unit tests should be fixed now. |
|
This PR is ready. It improves the usage of debian repositories a lot because features like Apt-Pinning can be used. It requires Katello/katello#10420 which we want to integrate in Katello soon. If everything is ready, we would of course ship now sub-man at https://apt.atix.de :) |
Since that PR still needs to be reviewed and eventually merged, accepting this right now would not be wise (the actual implementation required might change depending on the outcome of the katello PR). Hence, switching this back to draft until the katello PR is merged. |
hstct
force-pushed
the
parse-url-params-for-repositories
branch
from
79af61d to
71c8949
Compare
This change allows 'AptRepoFiles' to handle URL parameters for "Components" and "Suites" giving it more flexibility rather than always using a fixed default value.
quba42
force-pushed
the
parse-url-params-for-repositories
branch
from
71c8949 to
69df24e
Compare
| @@ -462,19 +462,28 @@ def sections(self): | |||
|
|
|||
| def fix_content(self, content): | |||
| # Luckily apt ignores all Fields it does not recognize | |||
| baseurl = content["baseurl"] | |||
| parsed_url = urlparse(content["baseurl"]) | |||
There was a problem hiding this comment.
In my development environment, I had to change this to:
| parsed_url = urlparse(content["baseurl"]) | |
| parsed_url = urlparse(unquote(content["baseurl"])) |
Maybe this is something that changed with a newer version of candlepin?
Perhaps we should make sure it will work both for quoted or unquoted baseurls!
There was a problem hiding this comment.
Using a recent Candlepin (4.4.x) by chance? If so, the problem should be a regression in CP, see also
https://community.theforeman.org/t/status-code-403-for-error-failed-to-download-metadata-for-repo-after-upgrade-to-3-11/38601/7
(CP developers are aware, unfortunately no public tickets to track this)
|
I have now opened a new updated PR for this here: #3454 @hstct Please close this PR in favor of my follow up PR. @ptoscano My apologies for the PR skip, we decided internally that it would be best if I take ownership of this PR, in addition to the related Katello PR Katello/katello#11058, so I can better pursue the finalization of both PRs. In my view, both PRs are ready for final review. They have already received significant internal review and testing at ATIX. I will keep updating them in case additional internal tests reveal any additional issues, but I don't expect major changes. If there is feedback from upstream reviewers, I will try my best to react in a timely manner. My hope is that this work is on the home stretch for getting merged. |
This change to subscription-manager handles parameters in the URL and thus giving us more options for handling different cases rather than set the values explicitly.
It is part of a change we are currently working on in Katello to replace Simple Publisher with Structured Publisher for Debian based repositories.
ref: https://projects.theforeman.org/issues/35959, Katello/katello#10420