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

Document the RPM copy API #1706

Merged
merged 1 commit into from May 20, 2020
Merged

Document the RPM copy API #1706

merged 1 commit into from May 20, 2020

Conversation

dralley
Copy link
Contributor

@dralley dralley commented May 13, 2020

]
dependency_solving=True

"Multi-repository-copy", required when any of the repositories involved in the copy are not "dependency closed".
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Explaining this in a satisfactorily simple way is quite difficult, but I'm sure there is still room for improvement.

Copy link
Member

Choose a reason for hiding this comment

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

I think you did a great job in this section. I can't think of a way describing it easier.

@pulpbot
Copy link
Member

pulpbot commented May 13, 2020

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

possible to install correctly on a client system.

3) A Module can depend on other modules. If those modules are not present in the RPM repo, the
module will not be installable on a client system.
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for the different cases (Module vs modules vs module)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just to draw more attention to the type that the bullet point is talking about. If it's distracting I can not do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, either way is fine.

docs/workflows/copy.rst Outdated Show resolved Hide resolved
@dralley dralley force-pushed the document-copy branch 5 times, most recently from 124a12f to c86b409 Compare May 14, 2020 14:19
Comment on lines 63 to 64
or Modules must be present in the same repository - otherwise the Advisory is effectively "broken"
and won't be possible to install correctly on a client system.
Copy link
Member

Choose a reason for hiding this comment

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

I'd say that otherwise the Advisory won't be fully installed on a client system.
It's possible to install it, dnf will install everything available, so only available subset will be installed.

Comment on lines 70 to 72
#. A Module consists of many RPM packages (similar to a Package Group). If the module is added to
a repository without its accompanying RPM packages, the module will not be installable on a client
system.
Copy link
Member

Choose a reason for hiding this comment

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

That's a completely valid statement and we can leave it here. Just to be clear, it's taken care of for any copy, not only for advanced version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I adjusted some of this language.

Comment on lines 83 to 84
* When copying a Module from one repository to another, all of its modular artifacts
(RPM packages) will also be copied.
Copy link
Member

Choose a reason for hiding this comment

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

See previous comment, it happens for any copy.

Comment on lines +114 to +125
Dependency solving does have some restrictions to be aware of. The set of content contained by
all repositories used in a copy operation must be "dependency closed", which is to say that no
content in any repository may have a dependency which cannot be satisfied by any content present
in any of the other repositories involved in the copy operation.

For example, in CentOS 8, there are two primary repositories which are called "BaseOS" and
"AppStream". RPMs present in the "AppStream" repository frequently depend on RPMs which are
not present in "AppStream", but are present in "BaseOS", instead.

In order to copy RPMs from a Pulp-clone of the "AppStream" repository, you must perform a
"multi-repository copy" so that the dependencies can be properly resolved. Please see the
recipe section below for more details on how to do this.
Copy link
Member

Choose a reason for hiding this comment

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

Should it have it's own heading? Like "Restrictions" or "Requirements". Up to you, just a suggestion to draw more attention to this section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't necessarily think so -- although I also don't know how to make a sub-heading this many levels deep.

]
dependency_solving=True

"Multi-repository-copy", required when any of the repositories involved in the copy are not "dependency closed".
Copy link
Member

Choose a reason for hiding this comment

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

I think you did a great job in this section. I can't think of a way describing it easier.


.. copy-workflow:

Advanced copy workflow
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth specifying config structure in the docs with the short description of each parameter there? Recipes will help to understand how to use them, however having a list of thing that you can possibly put into config might be useful. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually, I think so, but since there is only 5 of them at the moment and 3 of those are mandatory, I'm not sure if it's worth it for now.

@dralley dralley force-pushed the document-copy branch 3 times, most recently from d6ed547 to e3aecef Compare May 18, 2020 18:04
@dralley dralley merged commit b2fa257 into pulp:master May 20, 2020
@dralley dralley deleted the document-copy branch May 20, 2020 12:51
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.

None yet

4 participants