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

requirements: Remplacer pip-tools par uv #3682

Merged
merged 1 commit into from
Jul 31, 2024
Merged

requirements: Remplacer pip-tools par uv #3682

merged 1 commit into from
Jul 31, 2024

Conversation

rsebille
Copy link
Contributor

@rsebille rsebille commented Feb 16, 2024

pip-tools

CI : https://github.com/gip-inclusion/les-emplois/actions/runs/7928864910/job/21647976209

Local sur un nouveau venv :

$ time make venv
real    1m33.246s
user    1m0.090s
sys     0m4.402s

Local sur un venv déjà à jour :

$ time make venv
real    0m12.641s
user    0m12.205s
sys     0m0.425s 
$ time make compile-deps
real    2m35.001s
user    2m3.841s
sys     0m7.231s

uv

CI (install + sync) : https://github.com/gip-inclusion/les-emplois/actions/runs/7929274557/job/21649167500
Local :

$ time make venv  # without cache
real    0m17.898s
user    0m19.798s
sys     0m5.973s
$ time make venv  # with cache
real    0m5.368s
user    0m5.077s
sys     0m0.279s

CI (sync only) : https://github.com/gip-inclusion/les-emplois/actions/runs/7928945298/job/21648211811
Local :

$ time make compile-deps # without cache
real    0m11.897s
user    0m13.577s
sys     0m2.004s

$ time make compile-deps # with cache
real    0m0.331s
user    0m0.276s
sys     0m0.277s

@rsebille rsebille added the no-changelog Ne doit pas figurer dans le journal des changements. label Feb 16, 2024
@rsebille rsebille self-assigned this Feb 16, 2024
@vperron
Copy link
Contributor

vperron commented Feb 19, 2024

Tu as un copy/paste qui a foiré non ? Je comprends pas bien les 3 premiers.

@rsebille
Copy link
Contributor Author

@vperron Les 3 premiers c'est le temps sur ma machine avec un nouveau venv vide ou avec les dépendances déjà installées et à jour.

@francoisfreitag
Copy link
Contributor

Pas très motivé, tant qu’il n’y a pas le hash-checking mode : astral-sh/uv#474

J’aime bien vérifier ce que j’installe sur ma machine.

@francoisfreitag
Copy link
Contributor

Mais quand ce sera résolu, avec plaisir. C’est toujours un peu long de reconstruire le venv des emplois.

@xavfernandez
Copy link
Contributor

A réessayer avec https://github.com/astral-sh/uv/releases/tag/0.1.32

@rsebille
Copy link
Contributor Author

rsebille commented May 20, 2024

En local avec tout déjà à jour

$ time make venv  # pip-tools
real    0m5.043s
user    0m4.678s
sys     0m0.312s
$ time make venv  # uv
real    0m1.883s
user    0m1.763s
sys     0m0.115s
$ time make compile-deps  # pip-tools
real    0m53.257s
user    0m47.528s
sys     0m3.066s
$ time make compile-deps  # uv
real    0m2.055s
user    0m1.895s
sys     0m0.261s

Sur la CI

pip-tools, ~35-37s : https://github.com/gip-inclusion/les-emplois/actions/runs/9129608296/job/25104561687
uv, ~11s : https://github.com/gip-inclusion/les-emplois/actions/runs/9157037840/job/25172609344

@@ -1009,7 +1005,7 @@ psutil==5.9.8 \
--hash=sha256:d06016f7f8625a1825ba3732081d77c94589dca78b7a3fc072194851e88461a4 \
--hash=sha256:d16bbddf0693323b8c6123dd804100241da461e41d6e332fb0ba6058f630f8c8
# via py7zr
psycopg[binary]==3.1.10 \
psycopg==3.1.10 \
Copy link
Contributor

Choose a reason for hiding this comment

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

🤨 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

De ce que je comprend, uv a choisi de supprimer l'extra vu qu'au final tout est aplatis et qu'en le laissant on peux se retrouver à installer des choses non prévues mais aussi avoir des problèmes dans certain cas, mais il y a une option si besoin : astral-sh/uv/pull/2555.
Sachant que c'est déjà une discussion présente du coté de pip-tools (jazzband/pip-tools#1613) ou de pip pour le fichier de contrainte (pypa/pip#11599).

psycopg-binary étant l'extra derrière [binary] et étant bien présent je pencherais de ne pas avoir l'extra dans les .txt et seulement dans le .in.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Merci pour l’explication !

Makefile Outdated Show resolved Hide resolved
Copy link
Contributor

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

Autrement, ça a l’air top !

@rsebille
Copy link
Contributor Author

Vu suite à l'activation de dependabot coté pilotage : Support python uv as pip-compile compatible replacement.
Dans 90% des cas ça pose pas de problème, pour les 10 autres pourcent faut juste refaire un coup de make compile-deps / git amend sur la branche.

@francoisfreitag
Copy link
Contributor

Pour info, je l’utilise depuis quelques semaines sur un projet perso avec grand plaisir :)

@francoisfreitag
Copy link
Contributor

pour les 10 autres pourcent faut juste refaire un coup de make compile-deps / git amend sur la branche.

Je le fais déjà systématiquement pour les mises à jour de sécurité. 🤷

@rsebille
Copy link
Contributor Author

Bah je viens d'aller vérifier et c'était aussi sur une MAJ de sécurité, donc c'est tout comme avant en fait 😏.

Copy link

socket-security bot commented Jul 29, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
pypi/[email protected] eval, unsafe 0 2.91 MB mrabarnett

🚮 Removed packages: pypi/[email protected], pypi/[email protected], pypi/[email protected], pypi/[email protected], pypi/[email protected], pypi/[email protected]

View full report↗︎

@rsebille
Copy link
Contributor Author

L'erreur sur la CI est dû a la release récente (3 heures) de setuptools==72.0.0 : pypa/setuptools#4519

@rsebille rsebille changed the title requirements: Replace pip-tools by uv requirements: Remplacer pip-tools par uv Jul 29, 2024
@rsebille rsebille requested a review from a team July 29, 2024 07:47
@rsebille rsebille marked this pull request as ready for review July 29, 2024 07:48
Copy link
Contributor

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

C’est dommage d’avoir un expert pip dans l’équipe et de changer d’installateur, mais uv est tellement rapide et nous évite la dépendance à pip-tools qui me semble très peu actif.

👏

@xavfernandez
Copy link
Contributor

L'expert pip n'est plus si expert que cela 😬
Je suis carrément pour qu'on bascule sur uv 👍
Cela serait bien que dependabot/dependabot-core#10040 soit mergé mais au pire en attendant, on aura des commentaires qui bougent pour dire si pip-tools ou uv a été utilisé la dernière fois.

Copy link
Collaborator

@celine-m-s celine-m-s left a comment

Choose a reason for hiding this comment

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

🚀

@rsebille
Copy link
Contributor Author

@xavfernandez Doit y avoir un petit hack quelque part car les commentaires ne bougent pas, cf. les PR coté pilotage : https://github.com/gip-inclusion/pilotage/pulls?q=is%3Apr+is%3Aclosed+label%3Adependencies

@xavfernandez
Copy link
Contributor

@xavfernandez Doit y avoir un petit hack quelque part car les commentaires ne bougent pas, cf. les PR coté pilotage : https://github.com/gip-inclusion/pilotage/pulls?q=is%3Apr+is%3Aclosed+label%3Adependencies

Effectivement, dependabot semble garder le header intact: https://github.com/dependabot/dependabot-core/blob/f3cf9814094ca87840f4294de9e2928eb75ec68a/python/lib/dependabot/python/file_updater/pip_compile_file_updater.rb#L294

La PR dependabot/dependabot-core#10040 sert juste à utiliser uv vs pip-compile pour le cas où ils sortiraient des résultats différents ce qui a peu de chance d'être notre cas.

@francoisfreitag
Copy link
Contributor

Effectivement, dependabot semble garder le header intact: https://github.com/dependabot/dependabot-core/blob/f3cf9814094ca87840f4294de9e2928eb75ec68a/python/lib/dependabot/python/file_updater/pip_compile_file_updater.rb#L294

Je pense que c’est parce qu’utiliser la commande avec --package XXX (ce que fait dependabot) devrait changer le header pour capturer la ligne de commande avec --package XXX, ce qui causerait des changements de header à chaque PR de dependabot.

@rsebille rsebille added this pull request to the merge queue Jul 31, 2024
Merged via the queue into master with commit e8f502a Jul 31, 2024
11 checks passed
@rsebille rsebille deleted the rsebille/uv branch July 31, 2024 12:51
leo-naeka added a commit that referenced this pull request Aug 6, 2024
leo-naeka added a commit that referenced this pull request Aug 6, 2024
leo-naeka added a commit that referenced this pull request Aug 6, 2024
leo-naeka added a commit that referenced this pull request Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Ne doit pas figurer dans le journal des changements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants