-
Notifications
You must be signed in to change notification settings - Fork 4
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
Introduce UV to replace Pipenv; multi-stage Dockerfile #612
base: develop
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #612 +/- ##
========================================
Coverage 81.07% 81.07%
========================================
Files 130 130
Lines 5876 5876
========================================
Hits 4764 4764
Misses 1112 1112
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I assuming this is faster/more reliable/easier to maintain with uv than with pip/pipenv.
Just had a few questions/minor requests.
FROM ubuntu:noble | ||
SHELL ["sh", "-exc"] | ||
|
||
ENV PATH=$VIRTUAL_ENV/bin:/usr/local/bin:$PATH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checking - are all the environment variables (especially VIRTUAL_ENV) that were set during the build stage, also automatically set in the same way for the runtime stage? I guess so, since otherwise tests wouldn't work, but it seems surprising, since you don't really want some of the build env variables set in the runtime docker.
] | ||
|
||
[dependency-groups] | ||
dev = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like you dropped some packages under dev-packages in Pipfile, like asgi_lifespan, black, detect-secrets, and flake8. Just checking, was that on purpose?
[dependency-groups] | ||
dev = [ | ||
"docker", | ||
"geopandas", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You dropped some comments from the Pipfile for geopandas, gfw_pixetl, pandas, and retrying. Maybe add those comments back her in this file?
Pull request checklist
Please check if your PR fulfills the following requirements:
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
Using pipenv
Issue Number: N/A
What is the new behavior?
Pipenv replaced with uv
Does this introduce a breaking change?
Other information