Skip to content
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

Opensuse advisory support #1642

Merged
merged 1 commit into from
Apr 8, 2020
Merged

Conversation

pavelpicka
Copy link
Contributor

@pavelpicka pavelpicka commented Mar 13, 2020

Support for opensuse advisories.
Raised createrepo_c dependecny to 0.15 version.
Extend Packages' repo_key_fields w/location_href.

closes #5829
https://pulp.plan.io/issues/5829

@pavelpicka pavelpicka changed the title Opensuse advisory support [WIP] Opensuse advisory support Mar 13, 2020
@pavelpicka pavelpicka force-pushed the 5829-opensuse-support branch 4 times, most recently from 3014760 to 2517848 Compare March 16, 2020 15:16
@pavelpicka pavelpicka changed the title [WIP] Opensuse advisory support Opensuse advisory support Mar 16, 2020
@@ -236,8 +236,8 @@ class UpdateCollection(BaseModel):
update_record (models.ForeignKey): The associated UpdateRecord
"""

name = models.TextField()
Copy link
Contributor

@dralley dralley Mar 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why nullable? Nullable textfields means you have to deal with both null or emptystring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opensuse doesn't use it so can be null not only empty. same as shortname
can be seen in update.xml in repo mentioned above

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How safe would it be to collapse it into an emptystring regardless?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it will be an empty string createrepo_c will generate empty element to xml back when publish/distribute, which I think is unnecessary

@pavelpicka pavelpicka changed the title Opensuse advisory support [WIP] Opensuse advisory support Mar 17, 2020
@pavelpicka pavelpicka changed the title [WIP] Opensuse advisory support Opensuse advisory support Mar 26, 2020
@pulpbot
Copy link
Member

pulpbot commented Mar 26, 2020

Attached issue: https://pulp.plan.io/issues/5829

@@ -185,7 +185,7 @@ class Package(Content):
# not part of createrepo_c metadata
is_modular = models.BooleanField(default=False)

repo_key_fields = ('name', 'epoch', 'version', 'release', 'arch')
repo_key_fields = ('name', 'epoch', 'version', 'release', 'arch', 'location_href')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dralley , @daviddavis? are you ok with that change until we figure out the relative path problem?
Or do you prefer to use location_href only?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably OK for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pavelpicka can you add a comment, that createrepo_c treats src and nosrc as src, so 2 packages like pkg-1.src.rpm and pkg-1.nosrc.rpm won't be saved in a repo version unless we add location_href among repo_key fields

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment the explanation for easier understanding why we are adding it.

@goosemania
Copy link
Member

@pavelpicka , does it require a specific createrepo_c version? do we need to update the requirements?

Why do you have re #5829 in the commit message? Are more PRs expected for this issue?

@pavelpicka
Copy link
Contributor Author

@goosemania correct, I need to update requirements to createrepo_c 0.15, will check the issue if it is only one.

CHANGES/5829.feature Outdated Show resolved Hide resolved
Comment on lines 188 to 189
# createrepo_c treats 'nosrc' arch (opensuse specific use) as 'src' so it can seems there are
# two same packages when their are not. By adding 'location_href' here we can recognize this.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...it can seem that two packages are the same when they are not.

Support for opensuse advisories.
Raised createrepo_c dependecny to 0.15 version.
Extend Packages' repo_key_fields w/location_href.

closes #5829
https://pulp.plan.io/issues/5829
@ipanova ipanova merged commit e3e68fa into pulp:master Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants