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

Update pydantic to 2.7.1 #2059

Merged
merged 1 commit into from
May 3, 2024
Merged

Update pydantic to 2.7.1 #2059

merged 1 commit into from
May 3, 2024

Conversation

mr-tz
Copy link
Collaborator

@mr-tz mr-tz commented Apr 30, 2024

I think any pydantic major updates were disabled in #1673, so hopefully this enables them again.

Checklist

  • No CHANGELOG update needed
  • No new tests needed
  • No documentation update needed

@mr-tz mr-tz requested a review from mike-hunhoff April 30, 2024 13:13
@uckelman-sf
Copy link
Contributor

Would you have a look at #2053 when dealing with upgrading pydantic? Presently, flare-capa is behind what flare-floss requires. If you upgrade to 2.7.1, you'll be upgrading past what flare-floss requires---which will continue to make the current versions of flare-capa and flare-floss incompatible with each other.

@mr-tz mr-tz requested a review from williballenthin May 2, 2024 08:15
@williballenthin
Copy link
Collaborator

williballenthin commented May 2, 2024

To @uckelman-sf point, I wonder if/how we should relax dependency versions to make it easier to use capa as a library. Since we currently pin each dependency down to the patch version, we force users that integrate capa as a library to use exactly the same version as we declare. This may not be compatible with all environments (eg. version not yet available in internal corporate environments, or conflicts with another library (like floss)).

We might consider locking the deps for when we do a standalone build, and perhaps our tests too. But we could relax the deps in pyproject.toml for library users. This would just specify min/max supported versions (or more like "you should use the max, but anything down to the min probably works too". We would not guarantee to test anything but the max).

I've seen other projects use requirements.txt to lock the deps for a reproducible build (generated with pip freeze or similar), and then pyproject.toml has dep specs more like pydantic>2.0.0,<=2.7.1.

I don't love the duplication of information and bit more complexity that we have to remember, so I think it's fine we didn't do this before. But now that users request relaxed deps, we should consider it.

We'll also have to consider how dependabot works with this setup. Can it fix up two places?

Thoughts @mr-tz @mike-hunhoff @uckelman-sf ?

@williballenthin
Copy link
Collaborator

(that probably should have been posted in #2053, sorry)

@uckelman-sf
Copy link
Contributor

uckelman-sf commented May 2, 2024

To @uckelman-sf point, I wonder if/how we should relax dependency versions to make it easier to use capa as a library. Since we currently pin each dependency down to the patch version, we force users that integrate capa as a library to use exactly the same version as we declare. This may not be compatible with all environments (eg. version not yet available in internal corporate environments, or conflicts with another library (like floss)).

We might consider locking the deps for when we do a standalone build, and perhaps our tests too. But we could relax the deps in pyproject.toml for library users. This would just specify min/max supported versions (or more like "you should use the max, but anything down to the min probably works too". We would not guarantee to test anything but the max).

There are in my opinion good arguments in favor of not setting upper bounds at all for libraries unless you know that newer versions don't work. (See https://iscinumpy.dev/post/bound-version-constraints/. The recommendation here is to do a patch release with an upper bound as soon as you have a known bad newer version of a dependency.)

Python doesn't give you any escape hatch when you need a version of some package P which conflicts with a constraint specified by one of your dependencies---but it does let you further constrain the versions of package P when you discover that you need to for some reason.

I wonder if the simplest thing would be to package the application and the library separately. That way, the application could depend on the library, but also pin the exact versions of dependencies that you want in its pyproject.toml, and then the library could provide only lower bounds for its own dependencies in the library's pyproject.toml.

@mr-tz mr-tz merged commit 824e852 into master May 3, 2024
28 checks passed
@mr-tz mr-tz deleted the mr-tz-patch-1 branch May 3, 2024 08:45
@mr-tz
Copy link
Collaborator Author

mr-tz commented May 3, 2024

I think relaxing makes sense per the linked article... @uckelman-sf can you propose these changes unless @williballenthin disagrees?!

@williballenthin
Copy link
Collaborator

great article @uckelman-sf very convincing.

For the initial cutover, should we set version mins based on each dep's current major version number? eg pydantic becomes pydantic>=2.0.0?

Also agree it would be nice to get @uckelman-sf credit if he's open to contributing a PR with the pyproject changes.

@williballenthin
Copy link
Collaborator

I wonder if the simplest thing would be to package the application and the library separately. That way, the application could depend on the library, but also pin the exact versions of dependencies that you want in its pyproject.toml, and then the library could provide only lower bounds for its own dependencies in the library's pyproject.toml.

I'm a little hesitant to split up the Python package at this point, especially if we can get by with putting the app lock specification in requirements.txt or similar. Let's investigate this in another thread and update CI with whatever we discover.

@uckelman-sf
Copy link
Contributor

I will look into making a PR; maybe later today if I can, but if not then next week sometime. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants