-
Notifications
You must be signed in to change notification settings - Fork 38
refactor: Upgrade to Pydantic v2 #1566
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.
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
base: master
Are you sure you want to change the base?
Conversation
859455c to
47951bb
Compare
47951bb to
03c9489
Compare
|
| Branch | refactor/pydantic-v2 |
| Testbed | ubuntu-22.04 |
🚨 1 Alert
| Benchmark | Measure Units | View | Benchmark Result (Result Δ%) | Lower Boundary (Limit %) |
|---|---|---|---|---|
| sync-v2 (up to 20000 blocks) | Latency minutes (m) | 📈 plot 🚷 threshold 🚨 alert (🔔) | 1.36 m(-20.56%)Baseline: 1.72 m | 1.54 m (113.29%) |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result minutes (m) (Result Δ%) | Lower Boundary minutes (m) (Limit %) | Upper Boundary minutes (m) (Limit %) |
|---|---|---|---|---|
| sync-v2 (up to 20000 blocks) | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 1.36 m(-20.56%)Baseline: 1.72 m | 1.54 m (113.29%) | 2.06 m (66.20%) |
517d775 to
7250d52
Compare
| db_basic_settings = db_settings.copy(deep=True, exclude={'features'}) | ||
| new_basic_settings = new_settings.copy(deep=True, exclude={'features'}) | ||
| db_settings: FeatureActivationSettings = FeatureActivationSettings.model_validate_json(db_settings_bytes) | ||
| db_basic_settings = db_settings.model_copy(deep=True, update={'features': {}}) |
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'd rather use exclude instead of update.
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.
It seems exclude is not available in Pydantic v2. I guess it is ok to simply replace it by an empty dict.
|
|
||
| class SideDagArgs(RunNodeArgs): | ||
| poa_signer_file: str | None | ||
| poa_signer_file: str | None = None |
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.
Can it really be None?
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1566 +/- ##
==========================================
- Coverage 86.33% 86.26% -0.07%
==========================================
Files 437 437
Lines 33645 33610 -35
Branches 5258 5259 +1
==========================================
- Hits 29047 28994 -53
- Misses 3594 3606 +12
- Partials 1004 1010 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
107d6c4 to
d0ac1fe
Compare
| token_name: Optional[str] = None | ||
| token_symbol: Optional[str] = None |
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.
Pydantic v2 requires default value to ignore missing parameters. Should these None be included?
d0ac1fe to
84527e6
Compare
Motivation
This PR upgrades the codebase from Pydantic v1 to Pydantic v2.
Acceptance Criteria
Checklist
master, confirm this code is production-ready and can be included in future releases as soon as it gets merged