search

anancarv / python-artifactory

Typed interactions with the Jfrog Artifactory REST API
MIT License
55 stars 50 forks source link

pydantic v2 support #140

Closed sigma67 closed 11 months ago

sigma67 commented 1 year ago

Is your feature request related to a problem? Please describe. pydantic v2 is released, but this project has an upper bound on pydantic<2.0.

This leads to incompatibilities with other dependencies. Plus, pydantic v2 is faster than v1, therefore a migration is a good idea.

Describe the solution you'd like Support either v1 and v2 or migrate fully to v2, depending on the complexity of migration.

Note that supporting both versions at the same time would be cumbersome, due to many deprecations in v2.

This also requires bumping email-validator to v2 and due to that dropping support for Python 3.6

Describe alternatives you've considered

Additional context

sigma67 commented 1 year ago

A simpler alternative for removing the upper bound (pydantic<2) without moving to the new API: https://docs.pydantic.dev/2.0/migration/#continue-using-pydantic-v1-features

@anancarv are you open towards this solution?

heliocastro commented 11 months ago

@sigma67 I hope @anancarv is ok with that, but i forked and refactored entirely the current code to use pydantic2 and python 3.9+ features. Included all copyrights, of course. Right now one test is failing (disabled) and i have 3 mypy errors that are mypy only, but pydantic passes ok. Is here https://github.com/heliocastro/python-artifactory

anancarv commented 11 months ago

Hey @sigma67 @heliocastro , Sorry for the delay. I don't mind for the fork @heliocastro . However, if you want to, I would be glad to integrate your changes in the current project. I've seen that you've closed your PR, you can re-open it and I'll review it. Just keep in mind that this is a breaking change, so it will probably released on the next major version 2.0.0.

heliocastro commented 11 months ago

I made huge modifications top bottom, so when i finish i will come back to here and you decide how it goes. Anyway, you can see all in my repo for now. Thanks for answering @anancarv

sigma67 commented 11 months ago

@heliocastro do you have a working state on one of the earlier commits like https://github.com/heliocastro/python-artifactory/commit/bbbc6ad657b3d74dc4d5555129d008e411c34f72 ? Perhaps we could merge that now to avoid one huge PR?

heliocastro commented 11 months ago

I basically done, with a single addition on download, the ability to flat download.

What i did:

I had two days to make it work to my project in my company, so a took the YakShaving too seriously. Up to you what to do. I really hope you. find some use of it

Here now: https://github.com/heliocastro/python-artifactory/actions/runs/6198530723

( ps. deploy workflow is disabled, i only deployed in my internal artifactory for our use)

sigma67 commented 11 months ago

Awesome, shall I submit a PR based on it? Feel free to do so yourself!

sigma67 commented 11 months ago

144

anancarv commented 11 months ago

Released