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

repositories: deprecate default option and introduce possibility to disable PyPI explicitly #7430

Closed

Conversation

radoering
Copy link
Member

@radoering radoering commented Jan 29, 2023

Related-to: #6713

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

Motivation

While reviewing #6879, I figured out that there is a huge inconsistence between the implementation and the documentation of the default source (originally posted in #6879 (review)):

implementation documentation
If there is no explicit default, PyPI is only the default if there is no primary, otherwise it's secondary. default is the highest priority. In other words, implicit PyPI always comes after primary but is not always the default. It's a bit difficult to find but it says that primary sources take precedence over PyPI and that the default source takes precedence over secondary sources. It does not clearly say but implies that if there is no explicit default, PyPI always is the default. If you combine everything, it seems primary sources should take precedence over the default source (even if it's not PyPI).

In my opinion, the documentation makes more sense, but changing the implementation to match the documentation is a breaking change because it puts the explicit default source from highest priority to lower than all primary sources.

Thinking a bit about the problem, I came to the conclusion that we do not really need to explicitly specify a default source if we had another option to disable PyPI. Currently, specifying a default source has two effects:

  1. It disables PyPI.
  2. It defines the source with the highest priority.

The second effect is not really required because you can achieve the same result by making the source the first one in pyroject.toml.

The first effect is kind of implicit and from an UI perspective probably shouldn't be coupled to a specific repository. That's why I think we should deprecate configuring sources as default and introduce an option to disable PyPI.

Implementation details

  • Deprecate default=true for sources
  • Introduce new command poetry source default:
    • Options: --enable-pypi, --disable-pypi
    • When executed without an option it shows the current state (enabled/disabled).

pyproject.toml:

[tool.poetry]
...
default-source-pypi = false

@radoering radoering force-pushed the deprecate-default-source branch from 9121ba1 to 0974a97 Compare January 29, 2023 12:58
@radoering radoering force-pushed the deprecate-default-source branch from 0974a97 to e12ddb5 Compare March 5, 2023 14:51
@radoering radoering marked this pull request as ready for review March 5, 2023 14:55
@radoering radoering force-pushed the deprecate-default-source branch from e12ddb5 to bbeae93 Compare March 5, 2023 20:39
@radoering radoering added impact/docs Contains or requires documentation changes area/sources Releated to package sources/indexes/repositories labels Mar 5, 2023
@github-actions
Copy link

github-actions bot commented Mar 5, 2023

Deploy preview for website ready!

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

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

@radoering radoering marked this pull request as draft March 5, 2023 21:09
…ble the implicit default source PyPI explicitly
Copy link

github-actions bot commented Mar 3, 2024

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 Mar 3, 2024
@radoering radoering deleted the deprecate-default-source branch November 24, 2024 12:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/sources Releated to package sources/indexes/repositories impact/docs Contains or requires documentation changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant