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

Feature/poetry 1.1.6 #34

Closed
wants to merge 2 commits into from
Closed

Feature/poetry 1.1.6 #34

wants to merge 2 commits into from

Conversation

zyv
Copy link
Contributor

@zyv zyv commented Apr 22, 2021

@marns93 könntest du dich bitte darum kümmern, ggfs. reassignen? Vielen Dank!

@zyv
Copy link
Contributor Author

zyv commented Apr 22, 2021

P.S. Bei mir war es unbedingt notwendig die Lock-Datei mit Poetry 1.1.6 zu generieren (und Locking hat fast 4-5 Minuten gedauert) - vermutlich Poetry 1.1.6 funktioniert nicht richtig mit Locks von Poetry 1.1.5 wegen git-Abhängigkeiten.

Könnt ihr das bestätigen, und wir sollen das reporten / bis 1.1.7 warten, oder es ist nur bei mir so?

@marns93
Copy link
Contributor

marns93 commented Apr 23, 2021

P.S. Bei mir war es unbedingt notwendig die Lock-Datei mit Poetry 1.1.6 zu generieren (und Locking hat fast 4-5 Minuten gedauert) - vermutlich Poetry 1.1.6 funktioniert nicht richtig mit Locks von Poetry 1.1.5 wegen git-Abhängigkeiten.

Könnt ihr das bestätigen, und wir sollen das reporten / bis 1.1.7 warten, oder es ist nur bei mir so?

Das mit den 4-5 Minuten kann ich nicht in Bezug auf Poetry 1.1.6 bestätigen. Ich habe in meiner neuen VM erst mit Poetry 1.1.5 ein poetry install gemacht, das hat ebenfalls so lange gedauert.

Nach dem Update auf 1.1.6 habe ich versucht unsere Tests laufen zu lassen und habe einen Fehler beim Import von rest_framework erhalten.

Nachdem ich das Lockfile ebenfalls neu erstellt habe, war das Problem bei mir behoben. Kann das bitte noch jemand bestätigen?
@jwolpers84 @sbickmann @felix11h @stohrendorf

Sollte es bei euch ebenfalls so sein, werden wir das reporten!

Poetry 1.1.5
real 4m34,087s
user 0m48,998s
sys 0m9,780s

Poetry 1.1.6
real 0m30,224s
user 0m45,556s
sys 0m7,592s

Poetry 1.1.6 mit neuem Lockfile generieren
real 0m51,191s
user 0m22,700s
sys 0m4,446s

@zyv
Copy link
Contributor Author

zyv commented Apr 23, 2021

As discussed today during our standup:

  1. It seems that the locking from scratch was always that slow on Poetry 1.1.x, therefore it's definitively not a regression in Poetry 1.1.6 as compared to Poetry 1.1.5
  2. My suspicion that Poetry 1.1.6 doesn't work correctly with locks from Poetry 1.1.5 for projects with multi-level git dependencies was unfortunately confirmed by @marns93

Therefore we can't update as is, because it will break our deploys and the deploys of other users until one relocks the project.

We need to come up with a minimal reproducer and report this to Poetry team, but if anyone else knows that the issue has been already reported or wants to help us reporting it, then please do and link the issue here.

@marns93
Copy link
Contributor

marns93 commented Apr 28, 2021

As discussed today during our standup:

1. It seems that the locking from scratch was always that slow on Poetry 1.1.x, therefore it's definitively not a regression in Poetry 1.1.6 as compared to Poetry 1.1.5

2. My suspicion that Poetry 1.1.6 doesn't work correctly with locks from Poetry 1.1.5 for projects with multi-level git dependencies was unfortunately confirmed by @marns93

Therefore we can't update as is, because it will break our deploys and the deploys of other users until one relocks the project.

We need to come up with a minimal reproducer and report this to Poetry team, but if anyone else knows that the issue has been already reported or wants to help us reporting it, then please do and link the issue here.

I found a issue with problems in nested dependencies, which I have updated with another reproducible example.
python-poetry/poetry#3956

If there is a fix, we will bump our buildpack.

@zyv
Copy link
Contributor Author

zyv commented Apr 28, 2021

I found a issue with problems in nested dependencies, which I have updated with another reproducible example.
python-poetry/poetry#3956

Many thanks, I hope that they will take notice of it.

@marns93
Copy link
Contributor

marns93 commented Jun 24, 2021

I found a issue with problems in nested dependencies, which I have updated with another reproducible example.
python-poetry/poetry#3956

Many thanks, I hope that they will take notice of it.

Fix is merged: python-poetry/poetry#4202
Waiting for new Poetry release.

@zyv
Copy link
Contributor Author

zyv commented Jun 28, 2021

Just tried with 1.1.7 and it seems that the problem is not fixed :(

@zyv
Copy link
Contributor Author

zyv commented Jun 28, 2021

The root cause has been identified (see #4202) and this will be fixed in the next bugfix release.

Note that you will to regenerate the lock file for the fix to take effect.

Also der Fix ist eigentlich nur mittelgeil @marns93 @mm-matthias @felix11h - man muss unbedingt bei alle Repos die Locks mit die neue Version komplett neu schreiben - ich hoffe, dass es danach mit 1.1.5/1.1.6 kompatibel bleibt.

Ich frage mich wirklich ob wir nicht einfach bis 1.2.0 auf 1.1.5 bleiben sollen o_O ? Ansonsten wir schreiben alle Locks jetzt neu und upgraden (was uns erstmal eigentlich nichts bringt), aber vorab sollte man netterweise irgendwie die Nutzer von diesen Buildpack informieren...

@ulgens
Copy link
Contributor

ulgens commented Sep 19, 2021

Hey guys, English please 🙂 What about 1.1.8?

@marns93
Copy link
Contributor

marns93 commented Sep 20, 2021

@ulgens I've tried the latest Poetry release 1.1.9 and the issue seems to be fixed.

But as @zyv mentioned you have to update all of your poetry.lock files in all repositories, otherwise this will break your deployment.

At least we should inform the user with a friendly message. Perhaps with a check of the modification date of poetry.lock?

Other suggestions?

@zyv
Copy link
Contributor Author

zyv commented Sep 20, 2021

@ulgens is there any special reason why you would want to upgrade to Poetry 1.1.9?

We need to discuss this internally, re-locking dozens of our repositories with 1.1.9 in lockstep doesn't sound like fun. Also, I wonder whether there is an easy way to detect the Poetry version, which produced the lock file. If yes, then I would just switch the default to 1.1.9 and force-fail the deployments otherwise with a corresponding explanation.

The only annoying part is that we can't of course check the locks recursively, so this solution will be imperfect, and insufficient in our case, although I don't know how many other users have similarly complex setups... maybe in practice nobody is as crazy as we are.

In the meantime, you could already switch your projects with the following command:

heroku config:set POETRY_VERSION=1.1.9

@marns93
Copy link
Contributor

marns93 commented Dec 23, 2021

Sorry guys for the long waiting time.
I've created a new PR #36 to upgrade to the latest Poetry version 1.1.12.

It will be merged soon.

@marns93 marns93 closed this Dec 23, 2021
@marns93 marns93 deleted the feature/poetry-1.1.6 branch February 18, 2022 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants