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

Use warnings module for running as root message #10792

Closed
wants to merge 1 commit into from

Conversation

ssbarnea
Copy link
Contributor

@ssbarnea ssbarnea commented Jan 13, 2022

Allows use of warnings module for runtime warning as this allows fine tuning using PYTHONWARNINGS variable, providing a workaround for #10556

Related: #10772
Closes: #10556

This allows use of warnings module for runtime warning
as this allows fine tuning using PYTHONWARNINGS
variable, providing a workaround for pypa#10556
@ssbarnea ssbarnea marked this pull request as ready for review January 13, 2022 14:07
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

I don't think it's reasonable to integrate warnings in just one place. It'd require a cross-project replacement in a consistent manner 🤷‍♂️

Also, this seems like a band-aid and a proper solution would be to use virtualenv/pipx anyway: https://hynek.me/articles/virtualenv-lives/.

@webknjaz webknjaz added C: error messages Improving error messages kind: workaround Is a workaround for a problem state: needs eyes Needs a maintainer/triager to take a closer look labels Jan 13, 2022
@ssbarnea
Copy link
Contributor Author

ssbarnea commented Jan 13, 2022

I don't think it's reasonable to integrate warnings in just one place. It'd require a cross-project replacement in a consistent manner 🤷‍♂️

Also, this seems like a band-aid and a proper solution would be to use virtualenv/pipx anyway: hynek.me/articles/virtualenv-lives.

Did you try to look for usage of warnings module inside pip? There are tons of warnings already used in most vendored dependencies. It is nothing new, is not as that module was never used, or tricky.

I think that virtualenv was already ruled out as an anti-pattern for containers.

@pradyunsg
Copy link
Member

pradyunsg commented Jan 13, 2022

I think that virtualenv was already ruled out as an anti-pattern for containers.

I'm sorry, did I miss the memo on this?

How is the OS Python in a container somehow to be treated differently from the OS Python outside a container?

@pradyunsg
Copy link
Member

Honestly, while I do appreciate that you've filed this PR, I'd like to keep the discussion consolidated in a single place.

Please see #10556 (comment), and keep any further discussions on this topic over there.

@pradyunsg
Copy link
Member

Did you try to look for usage of warnings module inside pip? There are tons of warnings already used in most vendored dependencies. It is nothing new, is not as that module was never used, or tricky.

It's not used in pip's own code, for the most part. If it is, that's likely a bug that we should fix. (I know of one instance, that I have a local fix for)

Overall, I don't think this is going to be merging, so I'll eagerly close this. I don't want to prevent my fellow maintainers from reopening this, if they feel differently, but I'd like to keep all the discussion on this topic consolidated in #10556.

@pradyunsg pradyunsg closed this Jan 13, 2022
@potiuk
Copy link
Contributor

potiuk commented Jan 17, 2022

I'm sorry, did I miss the memo on this?
How is the OS Python in a container somehow to be treated differently from the OS Python outside a container?

I wrote a blog post precisely about it https://potiuk.com/to-virtualenv-or-not-to-virtualenv-for-docker-this-is-the-question-6f980d753b46

I recommend everyone to read it.

Also, this seems like a band-aid and a proper solution would be to use virtualenv/pipx anyway: https://hynek.me/articles/virtualenv-lives/.

I referred to the (2014/2015) article in my blog post and explained (with all the reasoning) why the author had good intentions in 2014 but how Container world evolved since and many best practices and fetures were added to containers that change the perspective - many of the things that make it difficult to use virtualenv for containers were introduced long after the article has been written.

@pradyunsg - encouraged by your comments and doubts if your questions have been addressed I will add a little longer comment about it in #10556

@webknjaz
Copy link
Member

@potiuk looks like you forgot to address the main problem — that users tend to mindlessly install things on top of the package manager-controlled locations which introduces breaking or hardly detectable changes to other software in that system.
I understand that for some people it's okay to break things they don't invoke but this can easily backfire in unexpected ways. Frankly, usually those people don't understand own fault and then flood the support forums begging to debug their systems. So it's quite understandable why the maintainers wouldn't want to increase this burden.
And it's not even interactive vs non-interactive invocations — the container builders must understand the intimate details of how the internals works and most people don't.
Even if you accept that some things will get overridden in the system, you'll probably still have some leftover files in those folders coming from the previous install. Or replace things that are supported to be linked against the OS-provided libs with fat static binaries which also has security implications on top of everything.
If you want a proper, not a half-broken/baked, setup, why not use tools that are designed for that ecosystem — put things into the OS native packages.

@pradyunsg
Copy link
Member

Please keep any further discussions in #10556.

@pypa pypa locked as resolved and limited conversation to collaborators Jan 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: error messages Improving error messages kind: workaround Is a workaround for a problem state: needs eyes Needs a maintainer/triager to take a closer look
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an opt-out for the “running as root” warning
4 participants