Pydantic v2

[sigma67]

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] This change requires a documentation update

How has it been tested ?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

• [ ] Test A
• [ ] Test B

Checklist:

• [ ] My PR is ready for prime time! Otherwise use the "Draft PR" feature
• [ ] All commits have a correct title
• [ ] Readme has been updated
• [ ] Quality tests are green (see Codacy)
• [ ] Automated tests are green (see pipeline)

[sigma67]

@anancarv I submitted on behalf of @heliocastro .

I presume we'll need a major release, and need to downgrade the Python lower bound to 3.8 at least.

I'm willing to make the changes needed to get this merged.

[heliocastro]

Just careful with the new workflows, to not clash with upstream one. Thanks to do that

[anancarv]

@anancarv I submitted on behalf of @heliocastro .

I presume we'll need a major release, and need to downgrade the Python lower bound to 3.8 at least.

I'm willing to make the changes needed to get this merged.

Thanks for your contribution. It's a huge PR so it might take a few days to review it completely. In the meantime, I can already say that the Python version should be lowered to 3.8 ( If possible 3.7).

[heliocastro]

@anancarv I submitted on behalf of @heliocastro . I presume we'll need a major release, and need to downgrade the Python lower bound to 3.8 at least. I'm willing to make the changes needed to get this merged.

Thanks for your contribution. It's a huge PR so it might take a few days to review it completely. In the meantime, I can already say that the Python version should be lowered to 3.8 ( If possible 3.7).

I went direct to 3.10 due my needs, 3.9 is doable, 3.8 and 3.7 i think is forcing too much. Python 3.8 will be EOL in one year, and i think downgrade the library just to have support not worth it. Maybe a suggestions, keek version 1 proper released supporting older versions and version2 do the leap jump.

[heliocastro]

You need change more places than that one, including the tests to be applied to 3.9. pyproject need to have 3.9 added in the metadata

[sigma67]

pyproject need to have 3.9 added in the metadata

that's exactly my commit? feel free to point out which tests need to be changed

[sigma67]

I found the places you mentioned I think. Let me know if I missed anything

@anancarv for downgrade to 3.8 would need to fix the typing as well

[themysteq]

Hi guys, Why this PR changes so much inside models? removing i.e. "typing.List" in favor of "builtin.list" does not seems to be productive - the change should be as simple and minimal as possible to allow us to migrate to pydantic>=2. Any other changes should be added within next revisions.

The project has been rewritten in many places at the same time. There is a problem with "|" operand in Python 3.9. This PR also drops support for Python 3.8 while it is currently supported (only security issues btw) but it's still not EOLed

[sigma67]

I agree, I will try to revert most of the changes targeting python >= 3.9

[heliocastro]

Hi guys, Why this PR changes so much inside models? removing i.e. "typing.List" in favor of "builtin.list" does not seems to be productive - the change should be as simple and minimal as possible to allow us to migrate to pydantic>=2. Any other changes should be added within next revisions.

The project has been rewritten in many places at the same time. There is a problem with "|" operand in Python 3.9. This PR also drops support for Python 3.8 while it is currently supported (only security issues btw) but it's still not EOLed

As i mentioned before, i made this way on my repo because i had an immediate reason to use and i intentionally dropped support for Python < 3.9.

@sigma67 offered to pick my branch and bring upstream, and i say ok, as long keep my original credits. But i did not intended originally to push upstream, when i saw that a lot of things need to change.

I would love to help right now, but then, i already have three big OSS projects beyond my work to jump totally, but then, all my changes are public and there to pick.

[themysteq]

Okay! Now it's more clear :) I'll try to help you with that, maybe I'll cherry pick the most crucial changes to only directly migrate to Pydantic v2 and after that we will try to rebase this PR onto new master (some changes on this PR are still useful).

Yeah, we also rely on pyartifactory and directly on pydantic so lack of v2 support also bugs us.

[heliocastro]

Please revert it back the license removal. Is not appreciated and simply looks like an hostlile move takeover.

[anancarv]

Please revert it back the license removal. Is not appreciated and simply looks like an hostlile move takeover.

@heliocastro sorry for the misunderstanding. I asked on my review to only keep the LICENCE file at the root of the project instead of all the new licence headers that were added. Is it ok if we do it that way with both our names within it ? Otherwise, it's also ok to keep the headers. Let me know which solution you prefer.

[heliocastro]

Keep the headers. Is industry standard practice and respect the copyrights of developers

[sigma67]

This has nothing to do with the PR goal of restoring pydantic v2 compatibility. Your copyright is retained in LICENSE and pyproject.toml.

This is exactly what the MIT license requires and has nothing to do with a hostile takeover.

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

[sigma67]

@anancarv I think most concerns are addressed except for the changes to the models, if you have time to review those

[anancarv]

@anancarv I think most concerns are addressed except for the changes to the models, if you have time to review those

Reviewed

[anancarv]

@sigma67 all the pydantic-v2 related comments have been resolved. Only the comments about the pyproject.toml file are left and this PR is good to be merged. Thanks on the huge work you've done

[sigma67]

@anancarv I updated the README

I can't find anything else about pyproject.toml in this PR, did you mean README? Else please link the relevant comment for me :)

Also not sure about the missing project-token in the tests at the end, is this because the PR has no access to secrets?

[anancarv]

@anancarv I updated the README

I can't find anything else about pyproject.toml in this PR, did you mean README? Else please link the relevant comment for me :)

Also not sure about the missing project-token in the tests at the end, is this because the PR has no access to secrets?

@sigma67 I commented the issues

https://github.com/anancarv/python-artifactory/pull/144#discussion_r1347903074

https://github.com/anancarv/python-artifactory/pull/144#discussion_r1347913732

https://github.com/anancarv/python-artifactory/pull/144#discussion_r1347915009

https://github.com/anancarv/python-artifactory/pull/144#discussion_r1347920360

About the token, I think it's because you don't have it set up on your project. But when it will be merged, it should work as expected.

[sigma67]

Thanks for the pointers, no idea how I missed those threads. GitHub PR view is confusing sometimes. Should be all done now

[sigma67]

Great :) anything else needed before we can merge?

[anancarv]

Great :) anything else needed before we can merge?

I think it's ok. I'll merge and release today.

[sigma67]

@anancarv updated README and updated lockfile