Manage dependencies with PDM #250

vbaltrusaitis-reef · 2024-02-01T10:04:56Z

This is a draft PR for two reasons:

It depends on b2sdk fixing Invalid escape sequences in docstrings Backblaze/b2-sdk-python#458 . A fix is in progress there. Currently the dependency points to my fork of b2sdk on GitHub.
Before merging, Backblaze team needs to be notified of the changes in CI / nox so they can adjust for their internal tests.

Changes:

pdm is used to manage dependencies, test and build b2.
Docker image and bundles now use locked dependencies, providing consistency in tests vs publishing.
All tests use locked dependencies. There is an unfortunate consequence - b2sdk is no longer installed from git master in CI. It would be possible to install b2sdk from master on top of locked dependencies but there are tradeoffs here:
- If we use b2sdk from master for everything, there's no indication that CLI depends on unreleased features of b2sdk. If developed CLI features depend on new b2sdk features, we will need to make a commit that updates pyproject.toml/pdm.lock anyways before release.
- With pdm, it seems natural to pdm add b2sdk @ git+https://github.com/Backblaze/b2-sdk-python while developing a feature together with sdk.
- If we're concerned with ensuring that new b2sdk are not somehow breaking the CLI, it might be better to resurrect Run CLI tests in CI Backblaze/b2-sdk-python#260
- It would make sense to run tests on both latest release and master of b2sdk but that would blow up our already long-running workflow (I think tests are parallelized only up to some number of workers).

mjurbanski-reef · 2024-02-01T10:12:15Z

.readthedocs.yml

+  jobs:
+    post_create_environment:
+      - pip install pdm
+      - pdm export --format requirements --prod --group doc --output requirements-doc.txt --no-hashes


why no hashes?

If you have a non-PyPI dependency (like a dependency on a git repo), pip install -r requirements.doc will error out:

ERROR: Can't verify hashes for these requirements because we don't have a way to hash version control repositories: b2sdk@ git+https://github.com/vbaltrusaitis-reef/b2-sdk-python@ac1e1355f1aaac10a398618214021cc7ecb6a1eb from git+https://github.com/vbaltrusaitis-reef/b2-sdk-python@ac1e1355f1aaac10a398618214021cc7ecb6a1eb (from -r requirements-doc.txt (line 21))

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.

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

mjurbanski-reef · 2024-02-01T10:15:39Z

Dockerfile.template

+ENV PDM_BUILD_SCM_VERSION=${version}
+RUN --mount=type=bind,source=./b2,target=/b2/b2 \
+    --mount=type=bind,source=./pyproject.toml,target=/b2/pyproject.toml \
+    --mount=type=bind,source=./pdm.lock,target=/b2/pdm.lock \


at first I was wondering why not use wheel/tar, but it does make sense to use lock indeed 👍

Yeah, lock is half the point. I did consider (and try out) a wheel+requirements approach: install dependencies using exported requirement file (pdm export --no-self [...]) and then install a b2 wheel on top of that with --no-deps . This would get rid of pdm within the container, less files to copy, it's nice. But in this case, I feel like I should definitely export requirements with hashes (see comment about the --no-hashes flag in readthedocs config). However, this would fail the docker build when you have b2sdk from git as a dependency. And that makes for an unpleasant development experience in case you're trying out unreleased b2sdk features, you want to make a draft PR that uses them and see if all tests pass.

This was a bit of a dilemma, I feel that exported requirements + wheel is nicer in many ways.

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.

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.

noxfile.py

mjurbanski-reef · 2024-02-01T10:45:30Z

pyproject.toml

+dependencies = [
+    "argcomplete>=2,<4",
+    "arrow>=1.0.2,<2.0.0",
+    "b2sdk @ git+https://github.com/vbaltrusaitis-reef/b2-sdk-python",


this for sure has to be b2sdk>=1.29.0,<2 like it was before

we cannot have pip install b2 users install stuff from git after this PR

Yes, absolutely. I'll change this before merging and submit for a review once b2sdk SyntaxWarning problem is fixed.

nox -s make_release_commit would detect this dependency and complain. That's, unfortunately, not the same as preventing this from reaching master, but it's something.

Co-authored-by: Maciej Urbański <[email protected]>

Dockerfile.template

vbaltrusaitis-reef · 2024-02-02T18:01:41Z

In the last few commits:

Implemented multi-stage Docker build (as per https://pdm-project.org/latest/usage/advanced/#use-pdm-in-a-multi-stage-dockerfile).
Moved license-outputs.txt generation to within Docker build.
Updated pyproject.toml to point to b2sdk from PyPI instead of local repo.
Fixed some issues where pyinstaller binaries would show b2 version as <version>.editable.

Requesting re-review @mjurbanski-reef

mjurbanski-reef · 2024-02-02T19:53:53Z

Dockerfile.template

-# 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
+ENV PYTHONPATH=/b2/pkgs


/opt/b2 will be slightly better IMO as per https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch03s13.html , but do as you will

mjurbanski-reef · 2024-02-02T20:02:38Z

changelog.d/+pdm.infrastructure.md

@@ -0,0 +1 @@
+Use pdm for building, testing and managing dependencies.


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.

vbaltrusaitis-reef added 13 commits January 31, 2024 20:08

Build with PDM

8fc6e4d

Fix docker tests

fba1446

Minor fixes

e416ee2

Use pdm in push_docker GH action

03ff618

Check b2sdk dependency before making a release commit

639f957

Build the docs with pdm

9662aea

Describe pdm use in CONTRIBUTING.md

c6a20e2

Remove setup.py

b1e8028

In nox sessions install as non-editable

a54e375

Format with yapf

2cb2de1

Install editable by default

38e191d

Dump license before building docker

d352158

Add changelog entry

2a7c12d

mjurbanski-reef approved these changes Feb 1, 2024

View reviewed changes

Style fix

de594eb

Co-authored-by: Maciej Urbański <[email protected]>

mjurbanski-reef reviewed Feb 2, 2024

View reviewed changes

Dockerfile.template Outdated Show resolved Hide resolved

vbaltrusaitis-reef added 4 commits February 2, 2024 19:05

Multi-stage docker build

d9da353

Format with yapf

cae6b12

Update pdm.lock to match pyproject.toml

774919d

Fix bundling

0b95e45

vbaltrusaitis-reef marked this pull request as ready for review February 2, 2024 18:02

Merge remote-tracking branch 'backblaze/master'

92ca332

mjurbanski-reef approved these changes Feb 2, 2024

View reviewed changes

vbaltrusaitis-reef added 3 commits February 5, 2024 16:47

Improve changelog entry

7e69959

Install b2 in /opt/b2 in Docker image

82aec06

Merge remote-tracking branch 'backblaze/master' into pdm

3463fff

vbaltrusaitis-reef merged commit 0693535 into reef-technologies:master Feb 5, 2024
30 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Manage dependencies with PDM #250

Manage dependencies with PDM #250

vbaltrusaitis-reef commented Feb 1, 2024 •

edited

Loading

mjurbanski-reef Feb 1, 2024

vbaltrusaitis-reef Feb 1, 2024

mjurbanski-reef Feb 2, 2024

mjurbanski-reef Feb 1, 2024

vbaltrusaitis-reef Feb 1, 2024 •

edited

Loading

mjurbanski-reef Feb 2, 2024

vbaltrusaitis-reef Feb 2, 2024

mjurbanski-reef Feb 1, 2024

vbaltrusaitis-reef Feb 1, 2024

vbaltrusaitis-reef commented Feb 2, 2024

mjurbanski-reef Feb 2, 2024

mjurbanski-reef Feb 2, 2024

		@@ -0,0 +1 @@
		Use pdm for building, testing and managing dependencies.

Manage dependencies with PDM #250

Manage dependencies with PDM #250

Conversation

vbaltrusaitis-reef commented Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbaltrusaitis-reef Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbaltrusaitis-reef commented Feb 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbaltrusaitis-reef commented Feb 1, 2024 •

edited

Loading

vbaltrusaitis-reef Feb 1, 2024 •

edited

Loading