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

remove advice re seeding keyring backends #9164

Merged
merged 1 commit into from
Mar 16, 2024

Conversation

dimbleby
Copy link
Contributor

I can't make sense of the advice that this pull request removes: so this is an invitation either to remove it or to explain it!

Surely all that matters to poetry is that the relevant keyring backend is in its own virtual environment? What reason would there be for also seeding the project virtual environment with that backend?

this comes from #4086 so I guess I should tag @Darsstar, though of course three years later it would be completely reasonable to have forgotten or stopped caring.

even if right in principle the doc is not enormously helpful. As far as I am aware the imagined plug-in remains imaginary - and I would not know from reading this text what I was supposed to do anyway. So I might encourage removing it in any case.

@Darsstar
Copy link
Contributor

Darsstar commented Mar 15, 2024

Seeding the venv isn't relevent with the modern installer, but as long as the old installer is still supported it should probably be updated instead of removed.

Edit: while I never made a poetry plugin, I did create keyring-subprocess which has a virtualenv seeder since that also worked for the projects at work using pipenv...

@dimbleby
Copy link
Contributor Author

... because the pip-based installer is using pip from the project environment: and the issue is not how to get poetry to use the backend, it is how to get that pip to use the backend?

thanks, that just about makes sense.

given that the pip-based installer is now deprecated I would vote for removing this advice anyway - I am currently in a $WORK conversation where it is confusing folk, so I know the downside to it being there, and I think the number of people benefitting from it is very likely zero.

@radoering radoering enabled auto-merge (squash) March 16, 2024 14:28
@radoering radoering merged commit 75453c2 into python-poetry:main Mar 16, 2024
21 checks passed
@dimbleby dimbleby deleted the remove-seeder-talk branch March 16, 2024 14:30
neersighted pushed a commit to neersighted/poetry that referenced this pull request Mar 24, 2024
@neersighted neersighted added area/docs Documentation issues/improvements impact/docs Contains or requires documentation changes labels Mar 24, 2024
Copy link

github-actions bot commented Mar 24, 2024

Deploy preview for website ready!

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

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

radoering pushed a commit to neersighted/poetry that referenced this pull request Apr 22, 2024
neersighted pushed a commit that referenced this pull request Apr 22, 2024
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 Apr 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 impact/docs Contains or requires documentation changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants