Port to pydantic 2 #192

jlrobins · 2023-10-19T21:17:49Z

Upgrade pydantic to pydantic 2.4.2. Rewrite bits needing rewriting, with the assistance of the porting tool bump-pydantic.

Wrote new unit tests for the validators that I had to respell.

Whole test suite passes w/o any deprecation warnings:

pytest tests
================================================================================ test session starts ================================================================================
platform darwin -- Python 3.11.6, pytest-7.4.2, pluggy-1.3.0
rootdir: /Users/jlrobins/noteable/origami
configfile: pyproject.toml
plugins: anyio-4.0.0, asyncio-0.19.0, cov-4.1.0
asyncio: mode=Mode.AUTO
collected 32 items                                                                                                                                                                  

tests/e2e/api/test_files.py ..                                                                                                                                                [  6%]
tests/e2e/api/test_projects.py ..                                                                                                                                             [ 12%]
tests/e2e/api/test_spaces.py ..                                                                                                                                               [ 18%]
tests/e2e/api/test_users.py .                                                                                                                                                 [ 21%]
tests/e2e/rtu/test_execution.py ...                                                                                                                                           [ 31%]
tests/e2e/rtu/test_notebook.py .....                                                                                                                                          [ 46%]
tests/unit/test_sql_cells.py ..                                                                                                                                               [ 53%]
tests/unit/test_version.py .                                                                                                                                                  [ 56%]
tests/unit/models/test_files.py .                                                                                                                                             [ 59%]
tests/unit/models/test_notebook.py ....                                                                                                                                       [ 71%]
tests/unit/models/test_project.py .                                                                                                                                           [ 75%]
tests/unit/models/test_rtu_base.py ......                                                                                                                                     [ 93%]
tests/unit/models/test_space.py .                                                                                                                                             [ 96%]
tests/unit/models/test_user.py .                                                                                                                                              [100%]

================================================================================ 32 passed in 43.14s ================================================================================

jlrobins · 2023-10-23T14:30:24Z

origami/clients/rtu.py

@@ -98,15 +98,18 @@ async def inbound_message_hook(self, contents: str) -> RTUResponse:
        if isinstance(rtu_event, NewDeltaEvent):
            extra_dict["delta_type"] = rtu_event.data.delta_type
            extra_dict["delta_action"] = rtu_event.data.delta_action
-        logger.debug(f"Received: {data}\nParsed: {rtu_event.dict()}", extra=extra_dict)
+
+        if logging.DEBUG >= logging.root.level:


Don't make the logging call that includes dumping the model if our current logging level is lower than DEBUG.

jlrobins · 2023-10-23T14:32:04Z

origami/clients/rtu.py

    # callback function should be async and expect one argument: a FileDelta
    # Doesn't matter what it returns. Pydantic doesn't validate Callable args/return.
    delta_class: Type[FileDelta]
    fn: Callable[[FileDelta], Awaitable[None]]

+    def __init__(self, delta_class: Type[FileDelta], fn: Callable[[FileDelta], Awaitable[None]]):
+        # With pydantic2, raises: "TypeError: Subscripted generics cannot be used with class and instance checks"


Had to comment out this well meaning check, 'cause pydantic 2 base implementation using parameterized types is incompatible with issubclass.

Would happily be open to suggestion on respelling it to pivot off of, say, a constant in the class or something to get the same effect?

need to pair with you on this to better understand what's hpapening

Sure, whenevs. If left in, the isinstance() check now raises an exception 'cause isinstance() can't be used with parameterized types.

jlrobins · 2023-10-23T14:33:46Z

origami/models/rtu/base.py



 class BooleanReplyData(BaseModel):
    # Gate will reply to most RTU requests with an RTU reply that's just success=True/False
    success: bool


-class BaseRTURequest(BaseModel):
+class BaseRTU(BaseModel):


Make a base class for both RTU requests and responses to hold the newly respelled channel_prefix validator.

BaseRTURequest then becomes just a pass.

jlrobins · 2023-10-23T14:34:38Z

tests/e2e/rtu/test_execution.py

    await rtu_client.wait_for_kernel_idle()

-    queued_execution = await rtu_client.queue_execution('cell_1')
+    queued_execution = await rtu_client.queue_execution("cell_1")


new Black respelled the quote style here, sorry.

yeah all of tests/ needs to have black applied post-double-quote-policy-change

kafonek · 2023-10-23T17:53:06Z

Code looks good, tests look good. What do you have in mind for medium-term maintenance while testing out all downstream apps that use Origami, and whether they can support the pydantic 2.x bump? Do we leave this as an open PR and for regular updates to Origami we merge into main then update this PR?

jlrobins · 2023-10-23T18:00:38Z

Code looks good, tests look good. What do you have in mind for medium-term maintenance while testing out all downstream apps that use Origami, and whether they can support the pydantic 2.x bump? Do we leave this as an open PR and for regular updates to Origami we merge into main then update this PR?

I think/suggest we can leave this PR as a PR while I work on at least the next concrete downstream useing branch, probablt in Origanist. Then produce a build of Origamist that uses this, and y'all could hoist up onto nightly or whatnot and get happy things work good, then go ahead and merge / make new releases from both. Then having new release of this clears the way (or one big step closer) to then being able to work on gate branch with upgraded pydantic.

James Robinson added 13 commits October 19, 2023 17:04

Progress porting to pydantic 2

6df13ed

isort

1420646

Tests over BaseRTU

0ba9802

Test over File.construct_url

e89d8c4

Test over User.construct_auth_type.

e63bbaf

Test over Space.construct_url()

56efd78

Test project.construct_url

5b13527

Notebook model unit tests.

93fdd56

isort

e14498e

Remove unused import

ae96905

Another round of dependency updates

ef8707d

Test suite passing without warnings

cf2d481

lint

8474af4

jlrobins marked this pull request as ready for review October 23, 2023 14:26

Remove bump-pydantic, no longer needed

58cd581

jlrobins commented Oct 23, 2023

View reviewed changes

jlrobins requested review from kafonek and shouples and removed request for kafonek October 23, 2023 14:35

Merge branch 'main' into upgrade_pydantic

533c8b1

jlrobins requested a review from kafonek October 23, 2023 14:36

James Robinson added 5 commits October 23, 2023 10:38

Better changelog

e0502d1

Merge remote-tracking branch 'origin/main' into upgrade_pydantic

d85d35a

Merge changelog

c4e9aa8

Merge branch 'main' into upgrade_pydantic

0d87526

Move to new changelog section

fa69c5d

Kafonek and others added 4 commits October 23, 2023 14:39

clean up DeltaCallback class (not Pydantic anymore)

2576e1c

Merge branch 'main' into upgrade_pydantic

1c28928

Merge branch 'main' into upgrade_pydantic

bfaf6ee

Merge branch 'main' into upgrade_pydantic

fab8b64

kafonek approved these changes Nov 6, 2023

View reviewed changes

kafonek merged commit 583d25c into main Nov 6, 2023
6 checks passed

kafonek deleted the upgrade_pydantic branch November 6, 2023 19:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port to pydantic 2 #192

Port to pydantic 2 #192

jlrobins commented Oct 19, 2023 •

edited

Loading

jlrobins Oct 23, 2023

jlrobins Oct 23, 2023

kafonek Oct 23, 2023

jlrobins Oct 23, 2023

jlrobins Oct 23, 2023

jlrobins Oct 23, 2023

kafonek Oct 23, 2023

kafonek commented Oct 23, 2023

jlrobins commented Oct 23, 2023 •

edited

Loading

Port to pydantic 2 #192

Port to pydantic 2 #192

Conversation

jlrobins commented Oct 19, 2023 • edited Loading

jlrobins Oct 23, 2023

Choose a reason for hiding this comment

jlrobins Oct 23, 2023

Choose a reason for hiding this comment

kafonek Oct 23, 2023

Choose a reason for hiding this comment

jlrobins Oct 23, 2023

Choose a reason for hiding this comment

jlrobins Oct 23, 2023

Choose a reason for hiding this comment

jlrobins Oct 23, 2023

Choose a reason for hiding this comment

kafonek Oct 23, 2023

Choose a reason for hiding this comment

kafonek commented Oct 23, 2023

jlrobins commented Oct 23, 2023 • edited Loading

jlrobins commented Oct 19, 2023 •

edited

Loading

jlrobins commented Oct 23, 2023 •

edited

Loading