-
Notifications
You must be signed in to change notification settings - Fork 25
Pydantic v2 migration #1415
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: main
Are you sure you want to change the base?
Pydantic v2 migration #1415
Conversation
b2abba0 to
312bcb6
Compare
2895a36 to
45fca0a
Compare
| #! "relationships" are not in the response because of "ItemModel = ITEM_MODELS[doc["type"]]", need to find a way to fix this. | ||
| # assert ( | ||
| # len([d for d in response.json["item_data"]["relationships"] if d["type"] == "collections"]) | ||
| # == 1 | ||
| # ) |
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.
NB
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.
| global ITEMS_FTS_FIELDS | ||
|
|
||
| if not ITEMS_FTS_FIELDS: | ||
| ITEMS_FTS_FIELDS = get_items_fts_fields() | ||
|
|
||
| if not ITEMS_FTS_FIELDS: | ||
| raise ValueError("Cannot create text indices: no fields available for full-text search") |
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.
This is super error prone and hard to test -- better to just call the function everywhere
45fca0a to
9818765
Compare
datalab
|
||||||||||||||||||||||||||||
| Project |
datalab
|
| Branch Review |
ml-evs/bump-pydantic-from-scratch
|
| Run status |
|
| Run duration | 07m 48s |
| Commit |
|
| Committer | Matthew Evans |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
354
|
| View all changes introduced in this branch ↗︎ | |
9818765 to
41e8ea1
Compare
Remove accidental repo
Rework ITEMS_FTS_FIELDS lookup
Chatblock patch; set default values in block
- Reorder imports for better isolation - Additional test cases for schema functionality
41e8ea1 to
28e43c3
Compare
BenjaminCharmes
left a comment
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.
LGTM, everything working great ! 👍
|
I have played around doing everything I can think of and seems to be working well for me so far! |
Supersedes #1398 and #1285 which were stuck in merge/rebase hell