-
Notifications
You must be signed in to change notification settings - Fork 9
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
Manage dependencies with PDM #250
Changes from 13 commits
8fc6e4d
fba1446
e416ee2
03ff618
639f957
9662aea
c6a20e2
b1e8028
a54e375
2cb2de1
38e191d
d352158
2a7c12d
de594eb
d9da353
cae6b12
774919d
0b95e45
92ca332
7e69959
82aec06
3463fff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,9 +11,21 @@ LABEL build-date-iso8601="${build_date}" | |
|
||
WORKDIR /root | ||
|
||
# due https://github.com/moby/moby/issues/47021 we cannot have /root/.cache leftover as it causes random errors in CI | ||
RUN --mount=type=bind,source=${tar_path}/${tar_name},target=/tmp/${tar_name} \ | ||
pip install --no-cache-dir /tmp/${tar_name}[full] && rm -rf /root/.cache | ||
# In case we're installing from git repo | ||
RUN apt-get update -y && apt-get install git -y | ||
|
||
RUN pip install -U pdm | ||
|
||
ENV PDM_BUILD_SCM_VERSION=${version} | ||
RUN --mount=type=bind,source=./b2,target=/b2/b2 \ | ||
mjurbanski-reef marked this conversation as resolved.
Show resolved
Hide resolved
|
||
--mount=type=bind,source=./pyproject.toml,target=/b2/pyproject.toml \ | ||
--mount=type=bind,source=./pdm.lock,target=/b2/pdm.lock \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. at first I was wondering why not use wheel/tar, but it does make sense to use lock indeed 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, lock is half the point. I did consider (and try out) a wheel+requirements approach: install dependencies using exported requirement file ( This was a bit of a dilemma, I feel that exported requirements + wheel is nicer in many ways. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. multi-layered dockerfilerfile is the way to go long term, not necessary now How big image is after this change? our last b2 image was 170MB, so unless this one is much bigger (>250MB) then I feel we can easily try doing it later down the line. When using requirements.txt I tried doing multilayer, but other than it being more error prone, the image size didn't differ much at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked the image size and yes, it increased by a lot after these changes, to 300Mb. So I implemented a multi-stage build and now it's 146M. |
||
--mount=type=bind,source=./LICENSE,target=/b2/LICENSE \ | ||
--mount=type=bind,source=./README.md,target=/b2/README.md \ | ||
cd /b2 \ | ||
&& pdm sync --prod --group full --no-editable --global --project . \ | ||
# due https://github.com/moby/moby/issues/47021 we cannot have /root/.cache leftover as it causes random errors in CI | ||
&& rm -rf /root/.cache | ||
|
||
ENTRYPOINT ["b2"] | ||
CMD ["--help"] |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Use pdm for building, testing and managing dependencies. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think seeing at issue reported in other repo it is worth mentioning here that tests and other superfluous files have been removed from sdist tarball. |
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.
why no hashes?
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.
If you have a non-PyPI dependency (like a dependency on a git repo),
pip install -r requirements.doc
will error out:To be fair, non-PyPI dependencies shouldn't get into master (and definitely definitely not release), so this is only a convenience for testing. But for this use-case this seems pretty harmless, I'm now on the fence if I should remove
--no-hashes
.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.
don't think it matters much - this is run on readthedocs side afterall and only for docs generation;
so the worst case (i.e. someone hacked pypi depdency) our docs get defaced - but at that point it would very cheap price to pay for knowing one of our dependencies were hacked :D