Add support for synchronizing oracle repos using ULN #1896

ThikaXer · 2020-11-30T16:34:48Z

This PR relates to the following PR in pulpcore:
Add headers option is aiohttp client session #1041

pulpbot · 2020-11-30T16:38:07Z

WARNING!!! This PR is not attached to an issue. In most cases this is not advisable. Please see our PR docs for more information about how to attach this PR to an issue.

quba42 · 2020-12-01T10:10:25Z

pulp_rpm/app/models/repository.py

@@ -51,6 +51,12 @@ class RpmRemote(Remote):
    TYPE = "rpm"
    sles_auth_token = models.CharField(max_length=512, null=True)

+    uln_user = models.CharField(max_length=512, null=True)
+
+    uln_pwd = models.CharField(max_length=512, null=True)


I noticed there are username and password fields in the Remote base class from pulpcore.
My question is if those are in use for anything specific, or could we repurpose them for this change (instead of adding uln_user and uln_pwd)?

Also, why are they part of the Remote base class? Are they really needed for every type of remote in every plugin?!

@dkliban I wonder if you could give some input on this question. Given I just watched a headers field being added to the remote base class, I guess this base class contains anything that might be of interest to more than one type of remote. I presume the existing username and password fields are intended in the same way, and we could have users use those instead of adding uln_user and uln_pwd. Is that right?

quba42

Just some thoughts on the server URL fields. These are not meant to be set in stone, but a starting point for discussion.

pulp_rpm/app/models/repository.py

pulp_rpm/app/serializers/repository.py

pulp_rpm/app/downloaders.py

quba42 · 2021-03-04T13:34:02Z

@bmbouter Thanks for the hint, it turns out I forgot to pass on **kwargs to a super().get_downloader call.

quba42 · 2021-03-09T09:43:37Z

pulp_rpm/app/models/repository.py

+
+    TYPE = "uln"
+    uln_server_base_url = models.CharField(
+        max_length=512, default="https://linux-update.oracle.com/"


This is currently the final item on the list of things I think I could improve about this PR:

I feel a bit uncomfortable defaulting to some Oracle URL right in the DB. I could just not do that, make uln_server_base_url a required field in the serializer, and leave everything to the user, but that would not be very user friendly. I think 99.9% of the time, users are going to want to use a single instance wide uln_server_base_url, and based on my research (Oracle is the only one I can find in the wild using ULN for content delivery) 99.8% of the time the value will be https://linux-update.oracle.com/.

So I have two questions (@goosemania, @bmbouter, or any one else with relevant knowledge):

Is there some existing mechanism to let users configure instance wide default values?

Is there somewhere else I could default to this value other than directly within the DB?

Perhaps I could make it a django setting?
https://docs.pulpproject.org/pulpcore/settings.html#pulp-settings

quba42 · 2021-03-18T17:08:45Z

I have successfully retested the latest state locally.

goosemania · 2021-03-18T17:44:39Z

CHANGES/7905.feature

@@ -0,0 +1 @@
+Added the ability to synchronize Oracle ULN repositories.


I'd add here a note about new setting as well.

goosemania · 2021-03-18T17:46:10Z

Hey @quba42 , it looks good to me, thanks for all the improvements and for the unittests.

I think it's worth adding some docs around new configuration option and the new remote. What do you think?
It can be done in the follow up PR, not to delay merging this one even more.

goosemania · 2021-03-18T17:52:01Z

requirements.txt

+pulpcore>=3.10
 PyGObject~=3.22
 solv~=0.7.17
+aiohttp_xmlrpc


@dralley , @jlsherrill, @evgeni , JFYI, when this is released (likely pulp_rpm 3.10), it will bump the pulpcore version requirement to 3.10, and also it adds a new dependency (in case @evgeni will need to build it for rpm installations).

I will have to, but thankfully we have tools to catch that.

Thanks for the ping tho :)

goosemania · 2021-03-18T18:52:15Z

@quba42 , it would also be good if you could squash commits, so all those changes are never separated (especially the migration). Thanks

quba42 · 2021-03-19T10:31:40Z

I have squashed the commits and opened a follow on task for the documentation (https://pulp.plan.io/issues/8426).
Not sure why flake8 has suddenly changed it's linting verdict... I will look into this later.

goosemania · 2021-03-19T10:42:53Z

It seems to be a new version of flake8-docstrings and pydocstyle.
I do not see this issue in other repos I checked, so I guess what they fixed now exposed the problem here.
Unrelated to your changes, let me fix it and you can rebase.

goosemania · 2021-03-19T11:23:35Z

It seems to be a new version of flake8-docstrings and pydocstyle.
I do not see this issue in other repos I checked, so I guess what they fixed now exposed the problem here.
Unrelated to your changes, let me fix it and you can rebase.

#1959

goosemania · 2021-03-19T12:15:37Z

It seems to be a new version of flake8-docstrings and pydocstyle.
I do not see this issue in other repos I checked, so I guess what they fixed now exposed the problem here.
Unrelated to your changes, let me fix it and you can rebase.

#1959

@quba42 , #1959 is merged, please rebase, so the linter passes. Thanks

goosemania · 2021-03-19T15:14:23Z

@quba42 , thanks, it all looks good. I'm not approving now, so it is not merged.
We'll merge it after the release next week. The main reason to postpone this one is that you bump pulpcore version and there are some items we need to release which should be compatible with pulpcore 3.7.

fixes #7905 https://pulp.plan.io/issues/7905 Should also work for non-Oracle ULN repositories, tested using Oracle.

goosemania · 2021-03-29T11:22:25Z

The failure is unrelated to the changes in the PR. The CI issue have been addressed separately, see #1963. Merging.

goosemania

Great job, everyone!
Thanks

bmbouter · 2021-03-29T13:04:41Z

Thank you all!

dralley · 2021-12-01T02:13:25Z

@ThikaXer It looks like an aiohttp-xmlrpc change has broken this support. Any chance you could revisit?

#2186

This is the change that broke ULN: https://github.com/mosquito/aiohttp-xmlrpc/pull/48/files

quba42 · 2021-12-01T08:35:16Z

@m-bucher, @sbernhard, @Manisha15 One of us will need to "revisit" this issue.

m-bucher · 2021-12-17T19:20:59Z

@dralley, @quba42, @Manisha15 I created a draft-PR, which incorporates the necessary changes: #2198
Feedback welcome on whether it should be split into multiple PRs/Tickets 😄

ThikaXer mentioned this pull request Nov 30, 2020

Add headers option is aiohttp client session pulp/pulpcore#1041

Merged

ThikaXer force-pushed the uln branch 13 times, most recently from 5cd2f92 to a022c56 Compare December 1, 2020 09:54

quba42 reviewed Dec 1, 2020

View reviewed changes

pulp_rpm/app/models/repository.py Outdated Show resolved Hide resolved

pulp_rpm/app/serializers/repository.py Show resolved Hide resolved

quba42 reviewed Dec 1, 2020

View reviewed changes

pulp_rpm/app/downloaders.py Outdated Show resolved Hide resolved

m-bucher reviewed Dec 1, 2020

View reviewed changes

pulp_rpm/app/downloaders.py Outdated Show resolved Hide resolved

ThikaXer force-pushed the uln branch 3 times, most recently from 5928a45 to 9023acb Compare December 1, 2020 12:05

bastian-src reviewed Dec 1, 2020

View reviewed changes

pulp_rpm/app/downloaders.py Outdated Show resolved Hide resolved

pulp_rpm/app/downloaders.py Outdated Show resolved Hide resolved

pulp_rpm/app/downloaders.py Outdated Show resolved Hide resolved

pulp_rpm/app/downloaders.py Show resolved Hide resolved

ThikaXer force-pushed the uln branch 4 times, most recently from ee79b4e to 2106cf6 Compare December 4, 2020 08:50

ThikaXer marked this pull request as ready for review December 4, 2020 10:33

ThikaXer force-pushed the uln branch 2 times, most recently from edfb245 to 87ad605 Compare December 9, 2020 12:19

quba42 force-pushed the uln branch from 9c1e03c to b2b62c5 Compare March 4, 2021 13:31

quba42 force-pushed the uln branch 2 times, most recently from 38741fb to 9f1facd Compare March 9, 2021 08:36

quba42 reviewed Mar 9, 2021

View reviewed changes

quba42 force-pushed the uln branch 3 times, most recently from b1dd52e to 48ec83c Compare March 18, 2021 17:04

goosemania reviewed Mar 18, 2021

View reviewed changes

quba42 force-pushed the uln branch from 48ec83c to 6b4fcfd Compare March 19, 2021 10:22

quba42 force-pushed the uln branch from 6b4fcfd to 63dcbe2 Compare March 19, 2021 13:57

Add ability to synchronize Oracle ULN repositories

f36c484

fixes #7905 https://pulp.plan.io/issues/7905 Should also work for non-Oracle ULN repositories, tested using Oracle.

quba42 force-pushed the uln branch from 63dcbe2 to f36c484 Compare March 29, 2021 08:12

goosemania merged commit 3157ad1 into pulp:master Mar 29, 2021

goosemania approved these changes Mar 29, 2021

View reviewed changes

quba42 deleted the uln branch April 1, 2021 10:59

Add support for synchronizing oracle repos using ULN #1896

Add support for synchronizing oracle repos using ULN #1896

ThikaXer commented Nov 30, 2020 •

edited

pulpbot commented Nov 30, 2020

quba42 Dec 1, 2020

quba42 Jan 27, 2021 •

edited

quba42 Mar 4, 2021

quba42 left a comment

quba42 commented Mar 4, 2021

quba42 Mar 9, 2021

quba42 Mar 18, 2021

quba42 Mar 18, 2021

quba42 commented Mar 18, 2021

goosemania Mar 18, 2021

quba42 Mar 19, 2021

goosemania commented Mar 18, 2021

goosemania Mar 18, 2021

evgeni Mar 18, 2021

goosemania commented Mar 18, 2021

quba42 commented Mar 19, 2021

goosemania commented Mar 19, 2021 •

edited

goosemania commented Mar 19, 2021

goosemania commented Mar 19, 2021

goosemania commented Mar 19, 2021

goosemania commented Mar 29, 2021

goosemania left a comment

bmbouter commented Mar 29, 2021

dralley commented Dec 1, 2021

quba42 commented Dec 1, 2021

m-bucher commented Dec 17, 2021

		@@ -0,0 +1 @@
		Added the ability to synchronize Oracle ULN repositories.

Add support for synchronizing oracle repos using ULN #1896

Add support for synchronizing oracle repos using ULN #1896

Conversation

ThikaXer commented Nov 30, 2020 • edited

pulpbot commented Nov 30, 2020

Choose a reason for hiding this comment

quba42 Jan 27, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quba42 left a comment

Choose a reason for hiding this comment

quba42 commented Mar 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quba42 commented Mar 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goosemania commented Mar 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goosemania commented Mar 18, 2021

quba42 commented Mar 19, 2021

goosemania commented Mar 19, 2021 • edited

goosemania commented Mar 19, 2021

goosemania commented Mar 19, 2021

goosemania commented Mar 19, 2021

goosemania commented Mar 29, 2021

goosemania left a comment

Choose a reason for hiding this comment

bmbouter commented Mar 29, 2021

dralley commented Dec 1, 2021

quba42 commented Dec 1, 2021

m-bucher commented Dec 17, 2021

ThikaXer commented Nov 30, 2020 •

edited

quba42 Jan 27, 2021 •

edited

goosemania commented Mar 19, 2021 •

edited