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 code to use Pydantic V2 #636

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Migrate code to use Pydantic V2 #636

wants to merge 10 commits into from

Conversation

nesitor
Copy link
Member

@nesitor nesitor commented Oct 24, 2024

Related Clickup or Jira tickets : ALEPH-151

Self proofreading checklist

  • Is my code clear enough and well documented
  • Are my files well typed
  • New translations have been added or updated if new strings have been introduced in the frontend
  • Database migrations file are included
  • Are there enough tests
  • Documentation has been included (for new feature)

Changes

Used tool bump-pydantic to migrate Pydantic V1 to Pydantic V2. Also added some manual modifications to fix orphan issues.

How to test

Ensure that everything works as usual without issues.

@nesitor nesitor self-assigned this Oct 24, 2024
@nesitor nesitor mentioned this pull request Oct 24, 2024
Closed
6 tasks
Psycojoker
Psycojoker previously approved these changes Oct 24, 2024
View reviewed changes
Copy link
Collaborator

@Psycojoker Psycojoker left a comment

Choose a reason for hiding this comment

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

Apart from some questions, LGTM

src/aleph/schemas/api/messages.py Show resolved Hide resolved
src/aleph/schemas/api/messages.py Show resolved Hide resolved
tests/api/test_list_messages.py Show resolved Hide resolved
@nesitor nesitor force-pushed the andres-migrate_pydantic_v2 branch 3 times, most recently from b472e50 to 5d068b0 Compare October 24, 2024 18:16
@nesitor nesitor force-pushed the andres-migrate_pydantic_v2 branch 5 times, most recently from bd9a9d6 to 88436b9 Compare October 24, 2024 22:17
@nesitor nesitor marked this pull request as ready for review October 24, 2024 22:29
1yam
1yam requested changes Oct 28, 2024
View reviewed changes
Copy link
Collaborator

@1yam 1yam left a comment

Choose a reason for hiding this comment

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

Except the .model_dump_json() used on aiohttp.Client Response who is not a pydantic model.

Else LGTM

src/aleph/chains/indexer_reader.py Outdated Show resolved Hide resolved
src/aleph/chains/nuls2.py Show resolved Hide resolved
src/aleph/chains/tezos.py Outdated Show resolved Hide resolved
src/aleph/chains/tezos.py Outdated Show resolved Hide resolved
src/aleph/services/ipfs/service.py Outdated Show resolved Hide resolved
src/aleph/services/p2p/http.py Show resolved Hide resolved
@nesitor nesitor requested a review from 1yam October 29, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Psycojoker Psycojoker Awaiting requested review from Psycojoker

@1yam 1yam Awaiting requested review from 1yam

Requested changes must be addressed to merge this pull request.

Assignees

@nesitor nesitor

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nesitor @Psycojoker @1yam