Switch pip to use Warehouse by default #5215

dstufft · 2018-04-13T12:12:36Z

I've made this PR against release/10.0.0, I'd love to get this into the upcoming 10.0 release. I held off making this PR until we were 100% sure we were good to go on the PyPI side. Starting Monday we're going to be redirecting everything (and we've been running load tests where we start redirecting things in the interim to validate Warehouse).

The new code base is good to go, and this will let us get folks onto it without paying the cost of extra redirects sooner. It will also get pip 10 users onto the new code base for search, which we can't redirect and which is a much better experience.

I'm not tagging this as a release blocker, but @pfmoore if you don't have any objections, I'd love if you merged this prior to release.

pfmoore · 2018-04-13T12:24:18Z

I'm planning on releasing pip 10 tomorrow morning (less than 24 hours from now). I'd need some very strong assurances that there would be no issues caused by this - in particular in the 2 days before Warehouse goes live "for real". Also, there are some scary looking test failures here, which imply things may not be *quite" that simple.

So I'm currently inclined to say no. Reluctantly, as I understand your reasons for wanting this in pip 10, but I'd have been much happier if you'd at least submitted the PR a week or two ago (or better still, in time for the beta), so we had time to see the test results and assess the implications.

pfmoore · 2018-04-13T12:32:30Z

Hmm, looks like you're changing this PR faster than I can review it (I keep getting "Change is not part of this commit"). I'll leave it for a while, but I have very little time today to review this, so please get it into a state where it can be reviewed ASAP if you want a review before the 10.0 release :-(

dstufft · 2018-04-13T12:35:34Z

@pfmoore The test failures were just places I didn't catch a reference to the old URL in the assertions. (I updated the tests by searching for pypi.python.org, but some places were just looking for substrings of that in the test assertions). I've updated those tests (though there might still be some I didn't catch, I'm monitoring the test results). None of the test failures are changes required to the code, just changes required to fix the assertions.

But basically, on the PyPI side, we've already had multiple multi hour long instances of redirecting from https://pypi.python.org/simple/ to https://pypi.org/simple/. Regardless on Monday when we throw the switch on the PyPI side, this change will effectively "land" server side either way and everyone will be using Warehouse. The practical difference between landing this change and not landing this change, is whether all of the pip 10 users will have to have an extra network request (and thus a slower experience) or if they'll default to using the new URL out of the gate.

The risk here is pretty small, basically that for some reason we'd delay or cancel the launch of Warehouse on Monday due to something that would break pip-- but that is super unlikely because as I said, we've already been redirecting people from Legacy to Warehouse in hours long bursts to try and shake out any issues that might exist.

I'll stop amending commits and just add new commits to fix any additional test failures, that should fix the "change is not part of this commit" error message.

pfmoore · 2018-04-13T12:41:05Z

tests/functional/test_install_config.py

@@ -24,10 +24,10 @@ def test_command_line_options_override_env_vars(script, virtualenv):
    Test that command line options override environmental variables.

    """
-    script.environ['PIP_INDEX_URL'] = 'https://b.pypi.python.org/simple/'
+    script.environ['PIP_INDEX_URL'] = 'https://www.pypi.org/simple/'


What's going on here? You're changing a reference to an invalid URL (AFAIK, https://b.pypi.python.org/simple/ isn't a real place) to a valid one. That seems like it's going to change the test...

Ah right, I thought we still had the N.pypi.python.org aliases in place for legacy PyPI, forgot we removed them at some point.

I'm going to update this test to use https://example.com/simple/ to make it clear this is explicitly an invalid URL.

That sounds like a good idea. Although I wonder if the original intent of the test was to have a valid URL, if that b.pypi URL was valid once. Who knows, at this stage?

I suspect the intent was a valid URL, but that the way the test is written it doesn't matter. The test doesn't care if something gets installed or not, and it's just asserting against the debug output which will be there regardless.

pfmoore · 2018-04-13T12:42:53Z

tests/functional/test_install_upgrade.py

@@ -221,7 +221,7 @@ def test_uninstall_before_upgrade_from_url(script):
    )
    result2 = script.pip(
        'install',
-        'https://pypi.python.org/packages/source/I/INITools/INITools-'
+        'https://files.pythonhosted.org/packages/source/I/INITools/INITools-'


What's the reference to pythonhosted.org? Why are we depending on files hosted there, rather than on PyPI like we did previously? Who will be maintaining those files (and who even has access to them)?

I'd prefer to continue getting our test files from PyPI, if we're not bundling them with the test suite.

That's where Warehouse stores files, it doesn't store it on the same domain anymore because there is security risk in the browser. You'll see it common amongst sites that allow users to upload content to have two domains, the website's domain and the "user content" domain. For Warehouse, files.pythonhosted.org is the user content domain.

You can see Github announcing a similar change at https://developer.github.com/changes/2014-04-25-user-content-security/.

So this is still getting our test files from PyPI :)

Interesting. Is that documented anywhere? Presumably if we moved our hosting domain, that would break all our references - whereas in the old tests, we'd have to rename PyPI before our tests would break.

Ultimately, if the tests work, it's not a big deal right now, but it does have an impact on the maintainability of our tests in the long run. So while I'm OK with it as an expedient way to get the tests working in the context of this PR, I'd like a proper discussion longer term (maybe even relocating these files into the test directory - I'd rather we didn't have a network dependency we don't need).

Yea, we desperately need to break the reliance on a network dependency in our test suite :(

I'm not sure if we ever explicitly documented that change TBH, it's been like that for like 2 years in Warehouse (it's one of the very first bits of Warehouse I completed) so I don't really remember what was documented about it (if anything).

Heh, it was mostly a rhetorical question. I'd have been surprised if we had documented it, my impression of pythonhosted.org was that it was a mostly-obsolete documentation site, TBH.

Yea, I ended up re-using that domain because it was there and having pythonhosted.org be the place for stuff that's hosted by Python, but isn't produced by Python seemed reasonable. The docs are at https://pythonhosted.org/ while the files are at https://files.pythonhosted.org/.

pfmoore · 2018-04-13T12:45:00Z

tests/unit/test_index.py

+        ("https://pypi.org/something", [], False),
+        ("git+http://pypi.org/something", [], True),
+        ("git+https://pypi.org/something", [], False),
+        ("git+ssh://git@pypi.org/something", [], False),


These are dummy URLs, in the sense that we don't fetch from them (after all, we also use example.com). While I understand you probably just got them by search and replace, is this change necessary?

Nah, I can keep the old URLs. I just changed them for complete-ness sake.

Actually, if this isn't landing in 10.0.0 I'll likely just keep these, just because I think in N years we'll be scratching our heads about this in the same way we were with b.pypi.python.org :)

pfmoore · 2018-04-13T12:49:39Z

Um. I don't really think I want to risk pip 10 for the sake of performance of a redirect. As I said, this PR could easily have been created a week or two ago, pointing beta users to Warehouse but not affecting pip 9.0.3 users.

I've added some review comments, but even if you address them, I still think it's too late to put this in pip 10. (I'm fine with a quick 10.0.1 that's billed as purely to switch to defaulting to the new PyPI now that it's gone live. The timing's unfortunate, because it'll look rushed, but no matter which way we look at it, that's true :-( And it's better IMO to look rushed than to be rushed).

pfmoore · 2018-04-13T13:09:55Z

OK, I'm going to make the call and say no, sorry, this is too late for 10.0.

I appreciate that's a disappointment, but I don't have any more time to look at this today, and pushing the release back to Sunday (the only other real option) doesn't fit well with my available time over the weekend.

I am happy to work on a 10.0.1 release with this in, if you think that's worthwhile, once the dust has settled on 10.0.0. We can discuss timescales after the release.

dstufft · 2018-04-13T14:05:41Z

The last remaining test failure is a difference in search results from Legacy PyPI to Warehouse, going to see about resolving that there-- but that will start affecting master on Monday regardless.

pfmoore · 2018-04-13T20:51:41Z

One point - another important milestone will be the Python 3.7 release. Beta 4 (the final beta) is due 30th April. and we'll need to get the latest pip included in that. If we want a 10.0.1 we should keep that in mind.

pfmoore · 2018-04-16T14:40:58Z

@dstufft Looks like I'm going to need to do a 10.0.1 to fix the distlib issue - do you want this to go in that release as well? If so can you get it merged into master ASAP, as I don't want to end up in a situation where we're waiting on this before we can fix the upgrade issue on Windows... (If Vinay gets a new distlib release done, I'll try to do 10.0.1 sometime before the weekend).

dstufft · 2018-04-16T14:44:44Z

@pfmoore Yes. Did I address/answer your review concerns adequately? If so could you update your review? :) There's one test failure to deal with, but that's going to start failing on master here in 15 minutes or so too so I'll need to get that fixed either way.

Warehouse is launching today, so I'm not sure I'll get this done today, but I'll try to get it done this evening.

pfmoore · 2018-04-16T15:26:32Z

Yeah, I'm OK with the changes you made. I suspect we should probably have a comment somewhere as to what those pythonhosted.org references are about, but it's not enough that I want to block this on it.

I assume you'll fix the CI failures before merging as well...

pfmoore · 2018-04-16T15:27:04Z

And good luck with the Warehouse launch!

dstufft · 2018-04-16T20:01:31Z

Ended up just skipping the test, because the test no longer makes sense with Warehouse's current behavior. Added pypi/warehouse#3717 to decide if the new behavior is our new expected (in which case, we can remove the test) or if we're going to fix it (in which case, un skip it).

lock · 2019-06-02T13:06:43Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

dstufft added this to the 10.0 milestone Apr 13, 2018

dstufft requested a review from pfmoore April 13, 2018 12:13

dstufft force-pushed the warehouse branch 2 times, most recently from 89b6833 to 3901370 Compare April 13, 2018 12:30

pfmoore requested changes Apr 13, 2018

View reviewed changes

dstufft modified the milestones: 10.0, 10.0.1 Apr 13, 2018

dstufft changed the base branch from release/10.0.0 to master April 13, 2018 13:12

dstufft force-pushed the warehouse branch 2 times, most recently from 9210afe to 6c9444d Compare April 13, 2018 13:15

pfmoore mentioned this pull request Apr 16, 2018

Plan for 10.0 #4981

Closed

pfmoore approved these changes Apr 16, 2018

View reviewed changes

dstufft added 2 commits April 16, 2018 15:54

Switch pip to use Warehouse by default

275ab7c

Skip test due to Warehouse behavior change

b2c5c67

dstufft force-pushed the warehouse branch from 6c9444d to b2c5c67 Compare April 16, 2018 20:00

dstufft merged commit eaa2d0c into pypa:master Apr 16, 2018

dstufft deleted the warehouse branch April 16, 2018 20:55

vphilippon mentioned this pull request Apr 20, 2018

Update all pypi.python.org URLs to pypi.org jazzband/pip-tools#652

Merged

3 tasks

lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 2, 2019

lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch pip to use Warehouse by default #5215

Switch pip to use Warehouse by default #5215

dstufft commented Apr 13, 2018 •

edited

Loading

pfmoore commented Apr 13, 2018

pfmoore commented Apr 13, 2018

dstufft commented Apr 13, 2018

pfmoore Apr 13, 2018

dstufft Apr 13, 2018

dstufft Apr 13, 2018

pfmoore Apr 13, 2018

dstufft Apr 13, 2018

pfmoore Apr 13, 2018

dstufft Apr 13, 2018

pfmoore Apr 13, 2018

dstufft Apr 13, 2018

pfmoore Apr 13, 2018

dstufft Apr 13, 2018

pfmoore Apr 13, 2018

dstufft Apr 13, 2018

dstufft Apr 13, 2018

pfmoore commented Apr 13, 2018

pfmoore commented Apr 13, 2018

dstufft commented Apr 13, 2018

pfmoore commented Apr 13, 2018

pfmoore commented Apr 16, 2018

dstufft commented Apr 16, 2018

pfmoore commented Apr 16, 2018

pfmoore commented Apr 16, 2018

dstufft commented Apr 16, 2018

lock bot commented Jun 2, 2019

Switch pip to use Warehouse by default #5215

Switch pip to use Warehouse by default #5215

Conversation

dstufft commented Apr 13, 2018 • edited Loading

pfmoore commented Apr 13, 2018

pfmoore commented Apr 13, 2018

dstufft commented Apr 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pfmoore commented Apr 13, 2018

pfmoore commented Apr 13, 2018

dstufft commented Apr 13, 2018

pfmoore commented Apr 13, 2018

pfmoore commented Apr 16, 2018

dstufft commented Apr 16, 2018

pfmoore commented Apr 16, 2018

pfmoore commented Apr 16, 2018

dstufft commented Apr 16, 2018

lock bot commented Jun 2, 2019

dstufft commented Apr 13, 2018 •

edited

Loading