Skip to content

Conversation

@mroote
Copy link

@mroote mroote commented Feb 11, 2020

Adds a method to copy content from one Pulp repo to another. I noticed there is an open issue to add this ability and I ran into this limitation when attempting to manage Python dependencies between environments when using Pulp.

I'm not too familiar with this code base so any suggestions to improve are appreciated. Also still need to add some tests to the suite for this method.

Copy link
Contributor

@rohanpm rohanpm left a comment

Choose a reason for hiding this comment

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

In this library we try to avoid dealing with Pulp's raw dicts and instead model some classes around our higher-level use-cases; so, instead of accepting a raw 'criteria' dict and expecting the caller to know/research what kind of criteria is accepted by Pulp, we provide documented classes instead. We deliberately try to hide various Pulp implementation details.

This means:

  • instead of accepting a 'criteria' dict, it should accept Criteria objects (https://release-engineering.github.io/pubtools-pulplib/api/searching.html#pubtools.pulplib.Criteria). You could follow the way it was done for Repository.search_content, since search and copy are so similar -
    but we don't currently support any searches on the associations, only on the units.
    Not sure whether that meets your requirements. There's also a suggestion #64 for extending Criteria.
  • instead of accepting an 'override_config' dict, it should accept something more flags-like, supporting flags relevant to your use-case. What values do you want to provide for override_config now and for what purpose?

Meeting that point about Criteria would be a bunch of extra work (most likely needs #64 done first), but I may as well ask - are you willing to look into that?

There are a couple of other points too but it only makes sense to look into them if the design is settled:

  • the FakeClient implementation needs to support this too
  • needs automated tests

@rohanpm
Copy link
Contributor

rohanpm commented Oct 26, 2021

An alternative copy implementation, client.copy_content, was introduced in version 2.17.0.

@rohanpm rohanpm closed this Oct 26, 2021
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.

2 participants