GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — feat: Upgrade the pydantic from v1 to V2 by shuchu · Pull Request #3942 · feast-dev/feast

Conversation

@shuchu

@shuchu shuchu commented Feb 8, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:
Based on the requirements from the Feast users, we want to upgrade the Pydantic version to v2 (2.6) for the performance of Feast.

Here are a few important package version changes:
pydantic >= 2.0.0
great_expectations>=0.15.41 (the upper bound is removed. Its required by the pydantic v2.)

Since it changes many files in the codebase, I humbly ask our reviewers to review this PR carefully.

Which issue(s) this PR fixes:
Fixes #3778

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bennettandrews reacted with rocket emoji
All reactions
  • 🚀 1 reaction
shuchu added 4 commits February 7, 2024 19:40
@shuchu
feat: Update the pydantic version to v2.
3bb680c 
Signed-off-by: Shuchu Han <[email protected]>
@shuchu
feat: fix the python lint check errors.
8bb1759 
Signed-off-by: Shuchu Han <[email protected]>
@shuchu
feat: update the requirements.
583c726 
Signed-off-by: Shuchu Han <[email protected]>
@shuchu
feat: fix few type checking errors.
31c6992 
Signed-off-by: Shuchu Han <[email protected]>
@shuchu
shuchu requested review from a team, DvirDukhan, kevjumba, niklasvm, sfc-gh-madkins, sudohainguyen and younggyu-oh as code owners February 8, 2024 02:39
@shuchu shuchu mentioned this pull request Feb 8, 2024
Pydantic 2.0 support #3778
Closed
@shuchu
fix: downgrade the version of moto to moto<5.
78bf235 
Signed-off-by: Shuchu Han <[email protected]>
@sudohainguyen

sudohainguyen commented Feb 8, 2024

Copy link
Copy Markdown
Collaborator

Seems a lot to do at a time, can we leave some of them for another PR such as great expectations?

Let's target pydantic only first
shuchu reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@shuchu

shuchu commented Feb 8, 2024

Copy link
Copy Markdown
Collaborator Author

The upgradation of "fastapi" and "great-expectation" are required by Pydantic v2. There was a discussion over here: #3778

Also the "mypy", if I don't upgrade it, I can not past the lint check.

It is a little bit struggling as you can see. :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@shuchu

shuchu commented Feb 8, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator Author

by the way, I can downgrade the fastAPI and great-expectation versions by using the "from pydantic.v1 import" migration strategy. If you think this is better.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@shuchu

shuchu commented Feb 8, 2024

Copy link
Copy Markdown
Collaborator Author

I just found some issues of fastAPI. let me try to change the version back (for fastAPI and greate-expectation). The unit test didn't show me any errors. Only the vscode gives me some hints of errors.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sudohainguyen

sudohainguyen commented Feb 8, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

Great, we can get rid of GX later cause it needs some extra work for the API changes.

Regarding fastapi we should expect to be resolved in this issue #3938
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bushwhackr bushwhackr mentioned this pull request Feb 8, 2024
chore: Bumping fastapi + starlette #3938
Merged
picnic-sven
picnic-sven reviewed Feb 8, 2024
View reviewed changes

@picnic-sven picnic-sven left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Exciting PR 🚀 !

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread sdk/python/requirements/py3.10-requirements.txt
pyarrow==15.0.0
# via feast (setup.py)
pydantic==1.10.13
pydantic==2.6.1

@picnic-sven picnic-sven Feb 8, 2024

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Is there a specific reason for requiring 2.6.1? Otherwise, would you consider using something like 2.0.0 to allow for better compatibility for downstream users of this package?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@shuchu shuchu Feb 8, 2024

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I didn't specify a version in the setup.py, so it loads the latest one :p. let me find a "safe" version around 2.0.0. @sudohainguyen Harry, what's your opinion?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@sudohainguyen sudohainguyen Feb 8, 2024

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

the txt files are just for CI workflows 😄
installation from end users should be fine since we set the boundaries in setup.py

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@picnic-sven picnic-sven Feb 9, 2024

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Apologies, the excitement got the better of me. Thanks for the clear up 💪

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@tokoko

tokoko commented Feb 8, 2024

Copy link
Copy Markdown
Collaborator

@shuchu Can't we upgrade mypy first without touching anything else?
shuchu reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sudohainguyen

sudohainguyen commented Feb 8, 2024

Copy link
Copy Markdown
Collaborator

@tokoko 😆 just had a thought about it too, worth a try I believe
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@shuchu shuchu closed this Feb 8, 2024
@shuchu shuchu reopened this Feb 8, 2024
@shuchu

shuchu commented Feb 8, 2024

Copy link
Copy Markdown
Collaborator Author

@shuchu Can't we upgrade mypy first without touching anything else?

Okay, let me try. Do you have a specific mypy version requirement? I'm using ~=1.1.1 in this PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@tokoko

tokoko commented Feb 8, 2024

Copy link
Copy Markdown
Collaborator

Okay, let me try. Do you have a specific mypy version requirement? I'm using ~=1.1.1 in this PR.

No, just suggesting a way to split the PR up into smaller PRs.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sudohainguyen

sudohainguyen commented Feb 9, 2024

Copy link
Copy Markdown
Collaborator

hey @shuchu in #3938 the author has done the mypy upgrade part
mind having a look? once the PR merged, this PR can move forward with ease
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@shuchu

shuchu commented Feb 9, 2024

Copy link
Copy Markdown
Collaborator Author

Seems a great relief to me :) Thank you for letting me know, Harry! @sudohainguyen
sudohainguyen reacted with rocket emoji
All reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
zseta and others added 2 commits February 9, 2024 14:44
@zseta
docs: Add ScyllaDB as online store alternative (fixed DCO) (feast-dev…
bdce99d 
…#3944)
@job-almekinders
fix: Add conn.commit() to Postgresonline_write_batch.online_write_bat…
7d75fc5 
…ch (feast-dev#3904)
tokoko and others added 2 commits February 9, 2024 14:45
@tokoko
fix: Transformation server doesn't generate files from proto (feast-d…
d3a2a45 
…ev#3902)
@bushwhackr
chore: Bumping fastapi + starlette (feast-dev#3938)
4e450ad
@sudohainguyen

sudohainguyen commented Feb 11, 2024

Copy link
Copy Markdown
Collaborator

Seems a great relief to me :) Thank you for letting me know, Harry! @sudohainguyen

mypy upgrade is done, would you mind pushing this one?
shuchu reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@shuchu

shuchu commented Feb 11, 2024

Copy link
Copy Markdown
Collaborator Author

Seems a great relief to me :) Thank you for letting me know, Harry! @sudohainguyen

mypy upgrade is done, would you mind pushing this one?

Sure, let me merge and adjust the great-expectation version. :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
shuchu added 6 commits February 10, 2024 22:43
@shuchu
feat: Update the pydantic version to v2.
7a5f036 
Signed-off-by: Shuchu Han <[email protected]>
@shuchu
feat: fix the python lint check errors.
23017c4 
Signed-off-by: Shuchu Han <[email protected]>
@shuchu
feat: update the requirements.
15e4789 
Signed-off-by: Shuchu Han <[email protected]>
@shuchu
feat: fix few type checking errors.
0c12a0c 
Signed-off-by: Shuchu Han <[email protected]>
@shuchu
fix: downgrade the version of moto to moto<5.
e412890 
Signed-off-by: Shuchu Han <[email protected]>
@shuchu
fix: Fix several type check errors. Upgrade the mypy version.
0f5e838 
Signed-off-by: Shuchu Han <[email protected]>
@sudohainguyen sudohainguyen added the dependencies Pull requests that update a dependency file label Feb 11, 2024
@shuchu

shuchu commented Feb 11, 2024

Copy link
Copy Markdown
Collaborator Author

it's ready for reviews. :) @sudohainguyen
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sudohainguyen

sudohainguyen commented Feb 12, 2024

Copy link
Copy Markdown
Collaborator

looks conflicts were resolved incorrectly, think we should re-do on a new PR 😢
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@shuchu

shuchu commented Feb 12, 2024

Copy link
Copy Markdown
Collaborator Author

looks conflicts were resolved incorrectly, think we should re-do on a new PR 😢

I see the problems. let me redo it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@shuchu shuchu mentioned this pull request Feb 12, 2024
feat: Update the Pydantic from v1 to v2 #3948
Merged
@shuchu

shuchu commented Feb 12, 2024

Copy link
Copy Markdown
Collaborator Author

@sudohainguyen check the new PR. :) #3948
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sudohainguyen sudohainguyen closed this Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sfc-gh-madkins sfc-gh-madkins Awaiting requested review from sfc-gh-madkins
@DvirDukhan DvirDukhan Awaiting requested review from DvirDukhan
@sudohainguyen sudohainguyen Awaiting requested review from sudohainguyen sudohainguyen is a code owner
@niklasvm niklasvm Awaiting requested review from niklasvm
@kevjumba kevjumba Awaiting requested review from kevjumba
@younggyu-oh younggyu-oh Awaiting requested review from younggyu-oh
1 more reviewer
@picnic-sven picnic-sven picnic-sven left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

dependencies Pull requests that update a dependency file

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Pydantic 2.0 support

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@shuchu @sudohainguyen @tokoko @picnic-sven @zseta @job-almekinders @bushwhackr
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.