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

Add a --parallel flag to the install command #2374

Closed
wants to merge 3 commits into from

Conversation

PetterS
Copy link
Contributor

@PetterS PetterS commented May 2, 2020

Tested this on our production codebase by installing from scratch two times:

Without this patch: 4 minutes and 22 seconds.
With --parallel: 1 minute and 8 seconds.

@PetterS
Copy link
Contributor Author

PetterS commented May 2, 2020

Can of course not be merged since it does not support 2.7 (actually seems to work in 2.7).

@PetterS
Copy link
Contributor Author

PetterS commented May 5, 2020

Tested this on our production codebase by installing from scratch two times:

Without this patch: 4 minutes and 22 seconds.
With --parallel: 1 minute and 8 seconds.

Worked really well!

@PetterS PetterS changed the title DRAFT: Add a --parallel flag to the install command Add a --parallel flag to the install command May 7, 2020
@ngryman
Copy link

ngryman commented May 26, 2020

👋 Hey folks, I just wanted to express explicit interest in this patch.

Our initial install times are floating around ~15 minutes. In comparison poetry export | pip install is around ~4 minutes. Unfortunately, the later one is 1) unreliable as pip regularly fails on "double requirements" errors and 2) should probably not become a widespread workaround to poetry install.

Is there anything we can do to bump the priority of this PR? Thanks in advance!

@PetterS PetterS marked this pull request as ready for review May 26, 2020 11:02
@PetterS
Copy link
Contributor Author

PetterS commented May 26, 2020

I don't think there is an explicit process for getting someone in the core team to look at a PR.

For example, the uncontroversial PR #2375 has been waiting for review for a long time now.

Myself, I am maintaining my own personal fork of Poetry with a couple of extra patches like this one. Would definitely prefer to merge something like this though.

@ngryman
Copy link

ngryman commented May 27, 2020

Hey @abn @finswimmer @sdispater @stephsamson, I'm explicitly raising your attention to this PR because I think it could really be beneficial for everyone.

We recently switched to use Poetry and we do love it! However our install times went from ~4 minutes to ~15 minutes which is starting to become a serious issue for us. These 10 more minutes quickly add up and our deployment pipelines and developer environment (we use Docker locally) got badly impacted performance-wise.

So now we're in this kind of situation where we do love the tool, but its performance is a big concern 😞

This PR cuts down install times by a factor of ~4 which is really a huge improvement 👏 . It seems that the change-set looks relatively straightforward, though I'm not an expert here. From my experience this kind of high-reward / low-risk PRs should be considered as high-priority since they don't involve that much time on the reviewer side.

Is there anything @PetterS or I could do to move things forward and help this PR to get merged?
Thanks in advance!

@sdispater
Copy link
Member

Thanks for your contribution!

There is an ongoing work (see develop...new-installer), to allow for parallel installations (with UI improvements to go with it) and other installation improvements, for instance downloading distributions separately.

The ultimate goal is to no longer rely on pip for the installation.

This will land in the next feature release as an experimental feature (activated with poetry config experimental.new-installer true). It's still in progress but I hope it will be available in the next alpha release of the 1.1 release.

@PetterS
Copy link
Contributor Author

PetterS commented May 27, 2020

That's nice!

@sdispater
Copy link
Member

Closing this PR since the PR for the new installer is now ready #2595

Thanks again for your contribution!

@sdispater sdispater closed this Jun 26, 2020
@PetterS
Copy link
Contributor Author

PetterS commented Jun 26, 2020

Looks really great!

Copy link

github-actions bot commented Mar 1, 2024

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 Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants