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

Modify the "you should upgrade" message to remind users to test #6443

Open
pfmoore opened this issue Apr 25, 2019 · 12 comments
Open

Modify the "you should upgrade" message to remind users to test #6443

pfmoore opened this issue Apr 25, 2019 · 12 comments
Labels
state: needs discussion This needs some more discussion type: maintenance Related to Development and Maintenance Processes

Comments

@pfmoore
Copy link
Member

pfmoore commented Apr 25, 2019

What's the problem this feature will solve?

Whenever pip releases a new version, we get people having issues because they pick up the new version in automated workflows, without having tested those workflows. As a result, feature changes or backward compatibility breaks have the potential to impact a lot of users with very little warning.

Describe the solution you'd like

Ideally, users would test their automated workflows before upgrading pip. It's obviously not possible to enforce this from pip (and indeed for many projects, such testing may be impractical), but it would be useful to encourage it by reminding people of the need to test before upgrading. This can be done in the "you should upgrade" message issued when pip detects that a new version is available.

Proposed addition to the message:

You are using pip version 19.0.3, however version 19.1 is available.
You should consider upgrading via the 'python -m pip install --upgrade pip' command.
Note that the new version may include breaking changes and as with any upgrade, you should test it before using in a production environment.

The change would be extremely low-cost (it is simply an update to a text message) and should itself have no compatibility issues. I will create a PR if the idea meets with general approval.

Alternative Solutions

The following options have proved ineffective in the past:

  1. General user education is difficult, as we don't really have a channel to contact all of our users.
  2. Beta releases have been tried in the past, but almost no-one was using them, and they add a non-trivial amount of overhead to the release process.
  3. Deprecation warnings are sometimes possible, but not all changes are amenable to deprecation warnings (and experience has shown that often users don't take much action in advance anyway).
  4. Stricter backward compatibility guarantees are difficult to handle, given the extreme lack of developer resource on pip - additional maintenance overhead is difficult to sustain. And they would also slow down progress to more modern packaging standards.
  5. Making new features and changes "opt in" doesn't seem to help much - people simply don't choose to "opt in" until forced to, so this simply delays the issue.

Additional context

This clearly won't solve the issue directly, and the suggestion may not actually be practical for many users. But even so, it may (over time) raise awareness of the risks, to the point where projects using pip at least have mitigation strategies in place for issues after an upgrade.

I am looking for general approval of the approach before implementing it as there is a risk that we will be seen as "lecturing" our users rather than addressing the breakage issues (even though that is emphatically not the intention).

@xavfernandez
Copy link
Member

xavfernandez commented Apr 25, 2019

For the record, the current message is:

You are using pip version 19.0.3, however version 19.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.

(I went looking for it)
(and the "python -m pip"/"pip" is only due to Windows/Linux difference)
(and maybe we could always suggest to use "python -m pip", even on Unix systems)

I don't feel strongly about this and mildly agree :)

@pradyunsg pradyunsg added the S: needs triage Issues/PRs that need to be triaged label Jun 22, 2019
@chrahunt chrahunt added state: needs discussion This needs some more discussion type: maintenance Related to Development and Maintenance Processes labels Jul 20, 2019
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label Jul 20, 2019
@deveshks
Copy link
Contributor

deveshks commented Apr 3, 2020

Hi @pfmoore , @xavfernandez

Was there any consensus made on the updating the language mentioned at the warning at https://github.com/pypa/pip/blob/master/src/pip/_internal/self_outdated_check.py#L232 . If yes, I can perhaps make a PR for it and add it to the repo.

I think we can also add this to the upgrade section https://github.com/pypa/pip/blob/master/docs/html/installing.rst#upgrading-pip to remind the user of what might be pre-checks before upgrading pip.

@uranusjr
Copy link
Member

uranusjr commented Apr 3, 2020

(and maybe we could always suggest to use "python -m pip", even on Unix systems)

IIRC there was a discussion on this, but was put on hold since python might not be a correct command either. I think the last suggestion in the discussion was to use {sys.executable} -m pip (we need to dig out that issue.)

@deveshks
Copy link
Contributor

deveshks commented Apr 3, 2020

Is it this issue @uranusjr #3488 ?

@uranusjr
Copy link
Member

uranusjr commented Apr 3, 2020

I thought there’s one @chrahunt was involved, but yeah the conclusion is the same as the one you mentioned as well. 🧐

@deveshks
Copy link
Contributor

deveshks commented Apr 3, 2020

Hi @uranusjr ,

So can I go ahead and just replace python -m pip with {sys.executable} -m pip as mentioned. In addition, do I also update the message as suggested in the first comment, and also update the doc as well?

@uranusjr
Copy link
Member

uranusjr commented Apr 3, 2020

Sounds good yo me. I would also suggest dropping the OS conditional and show {sys.executable} -m pip on all platforms, not just Windows.

@McSinyx
Copy link
Contributor

McSinyx commented Apr 3, 2020

Please consider @pfmoore's comments on the issue linked above:

At some point we have to assume the user has a basic understanding of what's going on, and simply strive to not be misleading. So maybe the best thing would be to deliberately not look like something the user can copy and paste

and

Honestly, I wonder whether we wouldn't be better not suggesting a command, but just saying "you should consider upgrading pip". There are just so many possible variants on what's the right thing to do (Windows has its own set of things that would go wrong with the current advice) that we're always going to get people making mistakes.

I don't really have an opinion on this though, just thought that it's worth reminding. Otherwise it might be a good idea to use {shlex.quote(sys.executable)} -m pip to ensure the command is runable.

Edit: see below, i.e. shlex.quote is not portable.

@deveshks
Copy link
Contributor

deveshks commented Apr 3, 2020

Hi @uranusjr

I just saw the code again, and we are already using {sys.executable} -m pip on all platform. Infact you yourself created that PR (#7532).

Given that's the case, should I still just update the message and update the doc as I mentioned ? Or should we leave things as-is and just close this issue?

@uranusjr
Copy link
Member

uranusjr commented Apr 3, 2020

😅 And #7376 was the issue I was looking for. I’ll defer to what @pfmoore thinks since the PR was created after this issue.

@pfmoore
Copy link
Member Author

pfmoore commented Apr 3, 2020

This issue is about adding "you should test" wording, not about whether we recommend pip install or python -m pip or whatever. I'm not really interested in the debate on what pip command we should recommend, but I am strongly against seeing this issue get sidetracked into that debate. So as far as I'm concerned, discussions over that part of the message are off-topic here.

As far as the key point of this issue is concerned, adding

Note that the new version may include breaking changes and as with any upgrade, you should test it before using in a production environment.

I don't believe there's any consensus yet. @xavfernandez said "I don't feel strongly about this and mildly agree :)", and none of the other pip committers have commented. My comment at the end of the issue:

I am looking for general approval of the approach before implementing it as there is a risk that we will be seen as "lecturing" our users rather than addressing the breakage issues (even though that is emphatically not the intention).

still applies - I don't feel sufficiently confident to make this change unilaterally, so it's basically on hold until someone wants to manage the discussion and get consensus on whether ths is worth doing.

(@deveshks if you want to manage that discussion, great! My opinion is still the same as I said above. I'd be happy to see other people comment - both pip committers and interested/affected users. But I don't have any great insight into how to get people's views 🤷‍♂).

@deveshks
Copy link
Contributor

deveshks commented Apr 3, 2020

Hi @pfmoore

Appreciate your insight, and I agree that making a change in the wording should be done with consensus of committers and users. I see that there is already a discussion needed label, so I would wait for others to chime in :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: needs discussion This needs some more discussion type: maintenance Related to Development and Maintenance Processes
Projects
None yet
Development

No branches or pull requests

7 participants