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

Update repositories.md #5605

Merged
merged 6 commits into from
Sep 18, 2022
Merged

Update repositories.md #5605

merged 6 commits into from
Sep 18, 2022

Conversation

jonapich
Copy link
Contributor

@jonapich jonapich commented May 13, 2022

Pull Request Check List

Resolves: some discussion in #3855

  • Added tests for changed code.
  • Updated documentation for changed code.

Clarifies default vs secondary (see discussion in python-poetry#3855)
@abn abn added the area/docs Documentation issues/improvements label May 13, 2022
@github-actions
Copy link

github-actions bot commented May 13, 2022

Deploy preview for website ready!

✅ Preview
https://website-hpwynp24n-python-poetry.vercel.app

Built with commit ec77eed.
This pull request is being automatically deployed with vercel-action

docs/repositories.md Outdated Show resolved Hide resolved
docs/repositories.md Outdated Show resolved Hide resolved
docs/repositories.md Outdated Show resolved Hide resolved
@jonapich jonapich requested a review from abn May 18, 2022 12:03
@jonapich
Copy link
Contributor Author

jonapich commented May 18, 2022

@abn I changed a few things. I added the --secondary to the first example too since that's what the wording was implying.

However, this part in the secondary source section is basically false:

All package sources (including secondary sources) will be searched during the package lookup process. These network requests will occur for all sources, regardless of if the package is found at one or more sources.

If you wish to avoid this, you may explicitly specify which source to search in for a particular package.

You do not "avoid this" by adding the source. If you add the source to a package, the other dependencies still use the secondary sources. I would rephrase to:

You can force a dependency to use a specific package source (exclusively? or it will look it up first and fallback? not sure) by specifying which source to search in for a particular package.

abn
abn previously requested changes May 19, 2022
docs/repositories.md Outdated Show resolved Hide resolved
[certificates](#certificates), please refer to the relevant sections below.
If you prefer to disable [PyPI](https://pypi.org) completely, you may choose to set one of your package sources to be the [default](#default-package-source).

To enable a package source only for a specific dependency, see [Secondary Package Sources](#secondary-package-sources).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
To enable a package source only for a specific dependency, see [Secondary Package Sources](#secondary-package-sources).
If the package source provides only specific dependencies, see [Secondary Package Sources](#secondary-package-sources).

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 think this suggestion makes it unclear what this is about. I rephrased to:

If you prefer to specify a package source for a specific dependency, see Secondary Package Sources.

docs/repositories.md Outdated Show resolved Hide resolved
@@ -171,7 +181,18 @@ If you wish to avoid this, you may explicitly specify which source to search in
package.
Copy link
Member

Choose a reason for hiding this comment

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

As per your comment, this maybe replaced like this.

- If you wish to avoid this, you may explicitly specify which source to search in for a particular package.
+ In order to limit the search for a specific package to a particular package source, you can explicitly specify what source to use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, I applied this, but is limit still true here? It is stated that:

All package sources (including secondary sources) will be searched during the package lookup process. These network requests will occur for all sources, regardless of if the package is found at one or more sources.

and:

If package sources are configured as secondary, all it means is that these will be given a lower priority when selecting compatible package distribution that also exists in your default package source.

So it seems like --secondary and source = my-secondary-index only means the search is prioritized there, and not limited there. However, I cannot test this, because my internal pypi server redirects me to pypi.org if a package is missing.

Copy link
Member

@neersighted neersighted Jun 4, 2022

Choose a reason for hiding this comment

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

source = should restrict the dep to only that repository -- if we're doing otherwise, that's a bug.

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 difficult for me to assert this, because my private pypi is set to redirect to pypi.org on missing packages.

Since this closely touches security, there really should be a unit test that asserts it.

@Secrus Secrus removed the status/waiting-on-response Waiting on response from author label May 19, 2022
jonapich and others added 3 commits May 24, 2022 10:58
Co-authored-by: Arun Babu Neelicattu <arun.neelicattu@gmail.com>
@jonapich jonapich requested a review from abn May 24, 2022 15:15
docs/repositories.md Outdated Show resolved Hide resolved
docs/repositories.md Outdated Show resolved Hide resolved
Co-authored-by: Bjorn Neergaard <bjorn@neersighted.com>
@jonapich jonapich requested a review from neersighted June 6, 2022 18:17
@neersighted neersighted merged commit a06381c into python-poetry:master Sep 18, 2022
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/docs Documentation issues/improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants