Fixes #37383 - Add OSTree Include/Exclude Refs options #10980
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lumarel
force-pushed
the
feature/ostree-incl-excl-properties
branch
from
06c7369
to
091a824
Compare
|
The VCR failures are related. We will need to re-record those. I can check if a re-record makes the tests green and if any updates are needed there. One question: Can we default the new remote options to null instead of [] and avoid the data migration on older remotes? |
|
Thanks for your review @sjha4 ! Hmm I tried switching the default to I'm also very happy for your help with the VCR failures, I think I once did such one for the Ansible modules, but has been a while 馃檪 |
|
Btw! Also found out that the changing the properties in the UI and them not getting promoted to pulp thing is a non-issue, the next time a sync gets triggered, it's definitely in the pulp remote config 馃憤馃徎 |
|
Ack..we refresh remote when a sync is triggered.
On the migration part, I am still not sure why we need it. Without that the remote options will be empty and that works for pulp. Do we need to set the include and exclude refs to blank at all. |
|
In my testing, I removed the migration and am able to sync older ostree repos as well as update the repo on the UI and sync it with refs etc. Only one VCR failure: katello/test/services/katello/pulp3/repository_integration_test.rb |
lumarel
force-pushed
the
feature/ostree-incl-excl-properties
branch
from
091a824
to
6caa241
Compare
|
@sjha4 Okay if you say it should work without the migration then I will of course remove it 馃檪 (removed the migration) |
sjha4
force-pushed
the
feature/ostree-incl-excl-properties
branch
from
55dd5e7
to
fbefa46
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are the changes introduced in this pull request?
Since Pulp OSTree plugin version 2.0.0 it's possible to use
include_refsandexclude_refsparameters,these are especially handy when you want to clone Fedora OSTree (Atomic Desktop) variants.
This PR adds theses 2
generic_remote_optionparameters.Considerations taken when implementing this change?
It was also necessary to do a db migration for already existing repos,
due to my very limited knowledge how to do that, my migration might look a bit bare minimum,
I would be very happy about a suggestion how to do that in a better way if there is one 馃檪
I also noticed that pulp takes the values on creation, but somehow in my test environment it only updated in the Katello db but not in pulp, couldn't find any errors or other code parts which would indicate that to happen. (will put in more time to find what's going on, but maybe it's already apparent to you what it is)I saw that there is the possibility that OSTree repos get added by a subscription, these shouldn't have anything in the
generic_remote_optionscolumn so the migration should also be fine there.What are the testing steps for this pull request?
Looks like this when set up:
