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

Jump to bottom

Ignore duplicate packages in metadata #1034

Merged
merged 1 commit into from May 6, 2017
Merged

Ignore duplicate packages in metadata #1034

merged 1 commit into from May 6, 2017

Conversation

fdobrovolny
Copy link
Contributor

We can now sync from repository which have the same package more than
once enlisted in the metadata.

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

@mention-bot
Copy link

@BrnoPCmaniak, thanks for your PR! By analyzing the history of the files in this pull request, we identified @mhrivnak, @jortel and @ipanova to be potential reviewers.

@pirxthepilot
Copy link

Hi there, was wondering in which version of Pulp will this merge be included? Thank you!

@dkliban
Copy link
Member

dkliban commented Mar 15, 2017

@pirxthepilot I believe it will be in 2.13

@ipanova ipanova self-assigned this Apr 19, 2017
ipanova
ipanova requested changes Apr 20, 2017
View reviewed changes
plugins/pulp_rpm/plugins/importers/yum/sync.py Outdated
@@ -1022,6 +1017,8 @@ def _identify_wanted_versions(self, package_info_generator):

:param package_info_generator: iterator of pulp_rpm.plugins.db.models.Package
instances
:param return_count: Retur number of unique packages parsed.
Copy link
Member

Choose a reason for hiding this comment

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

if it is a bool it cannot return 'number of unique packages'

plugins/pulp_rpm/plugins/importers/yum/sync.py Outdated
ret = {}
for units in wanted.itervalues():
for unit, info in units.itervalues():
ret[unit] = info

return ret
return (ret, number_of_unique_packages) if return_count else ret
Copy link
Member

Choose a reason for hiding this comment

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

since in this method the calculation of number of unique packages is ongoing anyway, i suggest to return always the tuple and ignore the second return value where it is needed, like in drpm case.
x, _ = return_multiple_values()

Copy link
Member

Choose a reason for hiding this comment

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

the above change would lead to removal of return_count parameter

plugins/pulp_rpm/plugins/importers/yum/sync.py
@@ -1080,6 +1082,8 @@ def _filtered_unit_generator(units, to_download=None):
# assume we want to download everything
yield unit
elif unit.unit_key_as_named_tuple in to_download:
# We don't need to download packages twice
to_download.remove(unit.unit_key_as_named_tuple)
Copy link
Member

Choose a reason for hiding this comment

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

niiice

@ipanova
Copy link
Member

ipanova commented Apr 20, 2017

@BrnoPCmaniak did you have a chance to test this with duplicate rpm/srpm/drpm?

@fdobrovolny
Ignore duplicate packages in metadata
4bc2615 
We can now sync from repository which have the same package more than
once enlisted in the metadata.

closes pulp#2605
https://pulp.plan.io/issues/2605
@fdobrovolny
Copy link
Contributor Author

Fixed but unable to test due to outage my vagrant fedora don't want to install.

@fdobrovolny
Copy link
Contributor Author

ok test

@coreaut
Copy link

coreaut commented Apr 26, 2017

@BrnoPCmaniak
I tested this patch with the pulp version 2.12.2

I still get the error:
Malformed repository: metadata is missing for some packages in filelists.xml and in other.xml

I tried to sync with:
http://resources.ovirt.org/pub/ovirt-4.0/rpm/el7/

@ipanova
Copy link
Member

ipanova commented Apr 28, 2017 •
edited

i tested against 2.12-dev with this PR

[ipanova@ina pulp]$ pulp-admin rpm repo create --repo-id=test-repo --feed=http://resources.ovirt.org/pub/ovirt-4.0/rpm/el7/

 Successfully created repository [test-repo]

[ipanova@ina pulp]$ pulp-admin -v rpm repo sync run --repo-id test-repo


+----------------------------------------------------------------------+
                  Synchronizing Repository [test-repo]
+----------------------------------------------------------------------+

This command may be exited via ctrl+c without affecting the request.


Downloading metadata...
[|]
... completed

Downloading repository content...
[|]
[                                                  ] 0%
RPMs:       52/899 items
Delta RPMs: 0/0 items

^C[ipanova@ina pulp]$

@Nebelwolf

@coreaut
Copy link

coreaut commented May 2, 2017

@ipanova i tested it only with pulp 2.12.2 release from https://repos.fedorapeople.org/repos/pulp/pulp/stable/2/7/x86_64/. Thanks

@ipanova ipanova removed their assignment May 3, 2017
@fdobrovolny fdobrovolny merged commit 637a008 into pulp:master May 6, 2017
@fdobrovolny fdobrovolny deleted the 2605-duplicate-metadata branch May 6, 2017 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ipanova ipanova ipanova approved these changes

Assignees
No one assigned
Labels
Bugfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@fdobrovolny @mention-bot @pirxthepilot @dkliban @ipanova @coreaut