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

build(poetry): use new (parallel) installer #2618

Merged
merged 1 commit into from
Nov 8, 2022
Merged

Conversation

prusnak
Copy link
Member

@prusnak prusnak commented Nov 7, 2022

We've been sticking to the old poetry installer because of python-poetry/poetry#3199

But it seems the issue is gone now (fixed in poetry 1.1.6) and we can use the new (parallel) installer again.

Change was introduced in #1436 and this PR reverts it

machinebeforeafter
Intel Xeon Desktop
$ time poetry shell
...
real	2m11.478s
user	1m43.056s
sys	0m9.334s
$ time poetry shell
...
real	0m32.622s
user	2m26.914s
sys	0m12.072s
Macbook Air M1
$ time poetry install
...
real	1m13.131s
user	0m52.100s
sys	0m7.698s
$ time poetry install
...
real	0m22.092s
user	1m14.373s
sys	0m12.048s

Obviously we'd need to test in the CI several times to be sure this works.

Revert "build: workaround poetry bug (KeyError)"

This reverts commit 6be367b.
@prusnak prusnak requested review from mmilata and matejcik November 7, 2022 22:26
@matejcik
Copy link
Contributor

matejcik commented Nov 8, 2022

don't we need to update poetry in nix-shell for this?

@prusnak
Copy link
Member Author

prusnak commented Nov 8, 2022

don't we need to update poetry in nix-shell for this?

No, we don't. We have poetry 1.1.13 there already, since we updated shell.nix in June - 0112d01. I was testing this PR using that version of poetry.

Btw, nixpkgs-unstable has poetry 1.2.2, I retested using that version:

machinebeforeafter
Macbook Air M1
$ time poetry install
...
real	0m57.129s
user	0m37.231s
sys	0m7.398s
$ time poetry install
...
real	0m21.188s
user	1m8.152s
sys	0m14.383s

The improvement is still significant for poetry 1.2.x.

Copy link
Contributor

@matejcik matejcik left a comment

Choose a reason for hiding this comment

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

ok then

@matejcik matejcik merged commit 4827cef into master Nov 8, 2022
@matejcik matejcik deleted the poetry-new-installer branch November 8, 2022 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants