Skip to content

Include pinned versions in pyproject.toml when necessary #2598

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.

Sign up for GitHub

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

Merged
mvandenburgh merged 2 commits into from
Oct 14, 2025
Merged

Include pinned versions in pyproject.toml when necessary #2598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
239ae1d
Include pinned versions in `pyproject.toml`
mvandenburgh Oct 10, 2025
44eaf6c
Remove comment about pinning
mvandenburgh Oct 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ dependencies = [
"celery",
"dandi", # minimal version is also provided in API /info
# Pin dandischema to exact version to make explicit which schema version is being used
"dandischema", # schema version 0.6.10
"dandischema==0.11.1", # schema version 0.6.10
Copy link
Member

@yarikoptic yarikoptic Oct 14, 2025

Choose a reason for hiding this comment

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

Note: I have concerns with such strict specification which were not resolved. Let's hope it would not immediately affect downstream testing anywhere etc.

Copy link
Member Author

@mvandenburgh mvandenburgh Oct 16, 2025

Choose a reason for hiding this comment

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

uv.lock is the source of truth for dependency installs, and dandischema is already set to 0.11.1 in there. So this shouldn't break anything in downstream projects.

I'm not super familiar with the release process/the way breaking changes and/or new features happen for dandischema, but generally we are very intentional about which version we're using. As opposed to, take django-allauth for example (just a random dependency - this applies to most of our other dependencies as well). That is a third-party dependency that we try to keep up to date for bugs, security fixes, etc, but we don't necessarily care about the changes in new releases that aren't applicable to dandi-archive. Whereas, dandischema development is typically driven solely by dandi-archive's requirements (and vice versa), so we usually upgrade that intentionally to pull in some new feature or schema change that dandi-archive needs.

And when doing dependency upgrades (like in #2588), my thinking is that dandischema should be treated differently and not get lumped in with third-party dependencies. I suppose another way to look at it is, while dandischema is a separate python library, it is essentially "our code", as opposed to a third-party package like django-allauth.

Copy link
Member

@waxlamp waxlamp Oct 16, 2025

Choose a reason for hiding this comment

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

I agree with @mvandenburgh about pinning dandischema. This is not the general case of wanting to absorb new features/enhancements and security fixes from upstream as with a lot of third-party dependencies; here we really do want to declare that the software works with this exact version of dandischema, and we're accepting the penalty of manual work to upgrade that dependency when that is necessary. (This is just another way of stating what Mike wrote above, that dandischema is "our code", and it's only inlined directly in the dandi-archive codebase solely because other components in the ecosystem need to use it.)

Put one more way: indeed, we want pyproject.toml to include acceptable ranges; for dandischema, the acceptable range is precisely one version.

Copy link
Member

@yarikoptic yarikoptic Oct 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

here we really do want to declare that the software works with this exact version of dandischema

that's exactly my point is that it actually works with many other versions of dandischema. As you both describe above -- it is just that our deployment ATM requires a specific one. But for instance development does not have that strict requirement (as e.g. we should be testing against another (development) version). Hence my comment that stating our strict deployment requirement (ok to be in uv.lock) as compatibility (what "works") requirement (what we state in pyproject.toml or setup.*) might give side-effects, as e.g. resolvers complain or refuse to use any other version while developing/testing etc.

"django[argon2]",
# Pin to version where this bug is fixed
# https://codeberg.org/allauth/django-allauth/issues/4072
"django-allauth",
"django-auth-style",
"django-click",
Expand All @@ -31,7 +29,7 @@ dependencies = [
"django-stubs-ext",
# TODO: pin djangorestframework until we figure out what the cause of
# https://github.com/dandi/dandi-archive/issues/1896 is.
"djangorestframework",
"djangorestframework<3.15.0",
"drf-extensions",
"drf-yasg",
"fsspec[http]",
Expand Down
4 changes: 2 additions & 2 deletions uv.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading