Cache trusted host

[NoahGorny]

Resolves #7847

This is impl second option discussed (in the issue), and adds a flag "cache-trusted-hosts"

• Add tests
• Figure out if first option is better (Change the behavior to always cache trusted-hosts)

[uranusjr]

I wonder whether something like --cache-insecure would be better. --cache-trusted-host sounds to me like we’re caching the --trusted-host values themselves.

[NoahGorny]

I wonder whether something like --cache-insecure would be better. --cache-trusted-host sounds to me like we’re caching the --trusted-host values themselves.

Thanks for the feedback!
I think that cache-insecure is also misleading, as we only cache hosts that are trusted (and not http for example).
I get your point though, but maybe the help section of the flag is enough to clarify it?

[uranusjr]

Hmm, you’re right, I guess there is not a perfect name (I can think of). Let’s go with the current one and see if anyone come up with something better before this goes in.

[NoahGorny]

Actually, @d3dave suggested in #7847 that it might be better to go for option 1 (changing the behavior to always cache trusted-hosts)

What do you think @uranusjr?

[hongyi-zhao]

@NoahGorny The merge is blocked, so I cannot test it.

Regards

[NoahGorny]

@hongyi-zhao Clone my forked repo, run pip locally (python src/pip) and use the flag --cache-trusted-hosts
See if it does cache the results this way

[hongyi-zhao]

@NoahGorny Cannot run it at all:

$ python src/pip install --cache-trusted-hosts six

Usage:   
  pip install [options] <requirement specifier> [package-index-options] ...
  pip install [options] -r <requirements file> [package-index-options] ...
  pip install [options] [-e] <vcs project url> ...
  pip install [options] [-e] <local project path> ...
  pip install [options] <archive url/path> ...

no such option: --cache-trusted-hosts

What's wrong with me?

[NoahGorny]

@hongyi-zhao Checkout to my branch as well
Git checkout cache-trusted-host
Then try again

Sorry I did not mention it before

[hongyi-zhao]

@NoahGorny Yes, it does the trick:

$ git checkout cache-trusted-host
$ python src/pip install --cache-trusted-hosts six
Looking in indexes: https://pypi.tuna.tsinghua.edu.cn/simple
Collecting six
  Using cached https://pypi.tuna.tsinghua.edu.cn/packages/65/eb/1f97cb97bfc2390a276969c6fae16075da282f5058082d4cb10c6c5c1dba/six-1.14.0-py2.py3-none-any.whl (10 kB)
Installing collected packages: six
Successfully installed six-1.14.0

But where can I find the local cached package file? I find nothing with the following command:

$ find ~/.cache/pip -type f -name 'six-1.14.0-py2.py3-none-any.whl'
$ 

Wish it can be merged into master.

Regards

[NoahGorny]

@hongyi-zhao I am glad this PR helped you 😄
About finding the wheel in the cache- take a look at #6391.

Also, what do you guys think about this PR @pradyunsg @uranusjr?
I saw at least two issues about this subject in the last week, and it affects me and my work mates as well.
Do you think it is a good option? or maybe we should change the default behavior as @d3dave suggested here?

[uranusjr]

I think it would be OK to change the current behaviour, but I am hard-wired to reject things that could weaken security. So my preference would be +1 on this behind a flag and good documentation, and +0 on automatically enabling it by default (deferring the decision to people who disabled that cache in the first place).

[dstufft]

This use case is probably not important enough to have a dedicated flag-- we should either just reject the proposal or just enable it entirely and say that opting in via --trusted-host is that flag.

[NoahGorny]

@dstufft I think the use case is pretty important given the fact that two independent issues were filed in the last week about this, just as I suffered from this. If dedicated flag is too much- than we should change --trusted-host behaviour, because leaving the situation as it is will continue to be problematic for many, at least in my opinion.

[pradyunsg]

I'm fine with doing this, since I guess trusting the host should only mean that pip skips TLS checks and treats the links from the host "as usual" otherwise.

I do not think we should have a flag, and this should be the default behaviour of the command.

[NoahGorny]

@pradyunsg thanks for the feedback 😃
I have changed the PR to now always allow caching trusted hosts (if caching is enabled of course)
You are welcome to take a look again :D

[NoahGorny]

@pradyunsg Any updates on this? I will have less time to work on this in the following weeks, so I want to be able to change and fix things in this PR if needed quickly

[NoahGorny]

Do you think we can merge it in time for #7951?
Me and my work mates would be very happy to see this change merged!

[hongyi-zhao]

Is this feature available or not now in the master branch?

[NoahGorny]

@hongyi-zhao As you can see in the bottom of this PR, this is not merged to master already.
You can see though that now it is part of the 20.1 milestone, which means it will probably be merged and be part of the next release soon 😄

[pradyunsg]

LGTM. I'll keep it open/un-merged for a few days, just in case @pypa/pip-committers or @pypa/pip-helpers wanna review this before it merges. :)

[pradyunsg]

Thanks @NoahGorny for the PR and everyone else for their inputs here! ^>^

[barakch]

Is this feature included in pip 20.1b1?
Because I tried to follow the steps mentioned here: #7847, with an http trusted host, and it seems not work.

[NoahGorny]

Is this feature included in pip 20.1b1?
Because I tried to follow the steps mentioned here: #7847, with an http trusted host, and it seems not work.

I did not add support for http trusted host. Maybe we should discuss this @pradyunsg (if not for this release, then for the next one)
This is a bit treaky, as you probably should set up https on your pypi server.

[barakch]

Got it. It is woking with https and invalid SSL certs when I'm adding the host as trusted.

[joaomarccos]

can you add suport to pip.conf too?

[deveshks]

can you add suport to pip.conf too?

I think you can create a new issue for that. Given that this is a merged PR, folks here might not get notified of your comment.