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.

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

Jump to bottom

Migrate models to pydantic 2 #17262

Merged
merged 182 commits into from
Jan 11, 2024
Merged

Migrate models to pydantic 2 #17262

merged 182 commits into from
Jan 11, 2024

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Jan 10, 2024

Continuation of #16527 ... sorry for the huge PR, but there's not much one can do incrementally.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

mvdbeek added 30 commits January 8, 2024 11:33
@mvdbeek
Temp
aae0376
@mvdbeek
Port OffsetNaiveDatetime to pydantic2
5673cbc
@mvdbeek
Fix up remaining pydantic issues
181c5e1
@mvdbeek
Remove DecodedDatabaseIdField instance check
c6a25b8
@mvdbeek
Replace DecodedDatabaseIdField.encode with Security.security.encode_id
1593570
@mvdbeek
Fix example -> examples
0fc981c
@mvdbeek
Fix valid_id validator
396206a
@mvdbeek
Fix validation error transform
e25553d
@mvdbeek
Drop explicit json encoder
b1e05c3
@mvdbeek
Fix ValidationError import
0c3c760
@mvdbeek
Adapt as_form decorator
8117f02
@mvdbeek
Drop more Required
727b21b
@mvdbeek
Fix one more ValidationError import
87a7b77
@mvdbeek
Also override validation schema
8987752
@mvdbeek
Revert me: drop custom schema generation
f49a399
@mvdbeek
Use model_construct and model_dump
82e1301
@mvdbeek
Field fixes
b55f335
@mvdbeek
Validation error fix
7f38e1c
@mvdbeek
Fix use of HistoryIDPathParam
76f17ab
@mvdbeek
Re-use more path params
e2475fd
@mvdbeek
Update user api path param
22ca53a
@mvdbeek
Fix double encoding of user ids
c2fa422
@mvdbeek
Rename id to workflow_id parameter
1472463
@mvdbeek
Fix one more case of __root__ in pydantic RootModel
49c5a9d
@mvdbeek
Optional attributes are still required
3fcc810
@mvdbeek
Don't double-encode user id
60d477c
@mvdbeek
Fix collection type optionality in collection payload
fbe3070
@mvdbeek
Create new type from annotated types
6cf2316 
That's needed so that the new type will be picked up from the default
value assignment, as in:

```
def needs_id(id: DecodedDatabaseId =Path(...))
```
@mvdbeek
Use model_rebuild instead of update_forward_refs
970bbf4
@mvdbeek
Fix fetch_data schema unit tests
ed1c30a
@mvdbeek mvdbeek added kind/refactoring cleanup or refactoring of existing code, no functional changes and removed area/dependencies area/libraries Data libraries area/objectstore area/testing/api area/client area/packaging labels Jan 10, 2024
@github-actions github-actions bot added this to the 24.0 milestone Jan 10, 2024
@mvdbeek mvdbeek requested review from a team and davelopez January 10, 2024 09:39
nsoranzo
nsoranzo reviewed Jan 10, 2024
View reviewed changes
lib/galaxy/schema/drs/openapi.yaml Outdated
@@ -0,0 +1,1731 @@
openapi: 3.0.3
Copy link
Member

Choose a reason for hiding this comment

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

Does this file need to be included in the repo?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah - expanded diff for the same question 😆.

nsoranzo
nsoranzo reviewed Jan 10, 2024
View reviewed changes
packages/schema/setup.cfg Outdated
@@ -31,7 +31,8 @@ version = 23.1.0.dev0
include_package_data = True
install_requires =
galaxy-util
pydantic[email]<2
pydantic[email]>=2
pycryptodome
Copy link
Member

Choose a reason for hiding this comment

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

I cannot find where this is used?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's used for id encoding and decoding via IdEncodingHelper

Copy link
Member Author

Choose a reason for hiding this comment

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

which I could make a type-only import

Copy link
Member

Choose a reason for hiding this comment

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

which I could make a type-only import

That's probably better, otherwise you'd need to require galaxy-data .

nsoranzo
nsoranzo reviewed Jan 10, 2024
View reviewed changes
packages/web_apps/setup.cfg Outdated
@@ -9,6 +9,7 @@ classifiers =
Natural Language :: English
Operating System :: POSIX
Programming Language :: Python :: 3
Programming Language :: Python :: 3.7
Copy link
Member

Choose a reason for hiding this comment

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

There seems to be a number of merge issues with this file, can you please double-check?

nsoranzo
nsoranzo reviewed Jan 10, 2024
View reviewed changes
scripts/dump_openapi_schema.py Outdated
AnyUrl,
url_regex,
)
from pydantic.networks import AnyUrl # url_regex,
Copy link
Member

Choose a reason for hiding this comment

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

Can the commented-out code be removed?

lib/tool_shed/webapp/api2/repositories.py Outdated
Comment on lines 511 to 512
def _hack_fastapi_4428(as_dict) -> RepositoryMetadata:
return RepositoryMetadata(root=as_dict)
Copy link
Member

Choose a reason for hiding this comment

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

This can probably just be dropped since it's called only in one place?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call, thank you!

jmchilton
jmchilton approved these changes Jan 10, 2024
View reviewed changes
Copy link
Member

@jmchilton jmchilton left a comment

Choose a reason for hiding this comment

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

This is wildly ambitious and I am grateful for our test coverage. Really, really impressive @mvdbeek.

@mvdbeek mvdbeek merged commit e341ced into galaxyproject:dev Jan 11, 2024
56 checks passed
@mvdbeek mvdbeek deleted the pydantic_v2_2024 branch January 11, 2024 08:36
This was referenced Jan 11, 2024
Closed
Closed
nsoranzo added a commit to nsoranzo/galaxy that referenced this pull request Jan 15, 2024
@nsoranzo
Update missed pydantic version pin
69d197e 
Forgotten in galaxyproject#17262 .

Fix broken Update dependencies CI build:
https://github.com/galaxyproject/galaxy/actions/runs/7509887325/job/20447594708
nsoranzo added a commit that referenced this pull request Jan 15, 2024
@nsoranzo
Merge pull request #17302 from nsoranzo/17274_followup
7af682d 
Follow-up on #17274 and #17262
@nsoranzo nsoranzo mentioned this pull request Jan 16, 2024
Closed
@davelopez
Copy link
Contributor

Amazing indeed! Thank you so much for taking care of all this 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nsoranzo nsoranzo nsoranzo left review comments

@jmchilton jmchilton jmchilton approved these changes

@davelopez davelopez Awaiting requested review from davelopez

Assignees
No one assigned
Labels
area/API kind/refactoring cleanup or refactoring of existing code, no functional changes
Projects
None yet
Milestone
24.0
Development

Successfully merging this pull request may close these issues.
4 participants
@mvdbeek @davelopez @jmchilton @nsoranzo