Skip to content

Conversation

@psychedelicious
Copy link
Contributor

@psychedelicious psychedelicious commented Aug 20, 2023

Experimental - add new nodes to pydantic models at runtime to allow for hot-loading of nodes.

Comment on lines +10 to +15
def reload_nodes(path: str, codefile: jurigged.CodeFile):
"""Callback function for jurigged post-run events."""
# Things we have access to here:
# codefile.module:module - the module object associated with this file
# codefile.module_name:str - the full module name (its key in sys.modules)
# codefile.root:ModuleCode - an AST of the current source
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, I gave you a function that will run after jurigged reloads anything. I haven't yet hooked it up to any of the update functions in this PR.

@keturn
Copy link
Contributor

keturn commented Sep 21, 2023

After our talk about Pydantic and its unions today, I had another go at this, and found it a surprisingly tough nut to crack. Brain dumping here:

We need two things to happen when we add a new invocation type:

  1. fastapi routes that accept any argument with a Graph type need to be able to accept the new Invocation as one of the elements of graph.nodes, able to deserialize it to the correct Python class.
  2. the OpenAPI schema advertised by fastapi needs to have a definition of that invocation type. (And it should appear in the list of permissible things to pass as a Graph node.)

I think the first one is easy. You can add validators to any Pydantic model, and though they're called "validator" that's where instantiation happens. It may be that all we need to do is pop in a validator that can create things of these late-discovered classes, instead of being limited to the ones known at time of first import.

The place I got stuck was on the schema. I figured there ought to be a way to override the schema generation too, maybe with some __modify_schema__ method, but I had a real bad time figuring out how to come up with something equivalent to how it usually writes the schema for fields with union types.

the problems

The parts that make this sticky are:

  • pydantic, at least in v1.10 that we're using, does a lot of work in metaclasses. That all runs when our classes are defined.
  • It's not set up for us to be able to re-run or re-configure it at will. e.g. ModelField.prepare says:

    Note: this method is not idempotent (because _type_analysis is not idempotent), e.g. calling it it multiple times may modify the field and configure it incorrectly.

  • When we do things like InvocationsUnion = Union[all_subclasses()] and then nodes: dict[str, InvocationsUnion], it doesn't just store a reference to InvocationsUnion in a way that will update if we mutate InvocationsUnion. It parses it and sucks out all the members of the union and uses them to set up all its internal fields in its initializers and prepare methods we can't re-run.
  • I thought I could run schema_of(Union[all_subclasses()]) to get sections of the schema to emulate its handling of union types, but naively calling schema_of in the middle of a schema-generation function is a ticket to infinite-recursion funtimes.
  • some prior discussion on this sort of use case is in Use subclasses of Abstract Base Class model for validation? pydantic/pydantic#1932 but without resolution.

other ideas

  • We can, at any time, create a new Graph class, and all its field definitions and validators will then be up-to-date, but I'm not sure how much good that does us when all the fastapi routes are built with references to the old version of the class. Maybe when publishing the schema, we can overwrite Graph's entry in definitions with one from a newly-regenerated Graph type?

@keturn
Copy link
Contributor

keturn commented Sep 21, 2023

many of these ideas get way deeper into the weeds of Pydantic internals than I wanted to be. It may be a bad investment to spend much more time on that with the Pydantic 1.x API; maybe it's something to schedule for after upgrading to the current versions of Pydantic + FastAPI.

@psychedelicious
Copy link
Contributor Author

Great detective work!

I agree that we should wait for pydantic v2 to spend too much time on this, unless it really strikes your fancy.

(not only are we on crusty pydantic v1, we are on fastapi 0.88 or something legitimately ancient - we are really due for an upgrade, but there are some puzzles to solve along the way to being up to date)

@lillekemiker
Copy link
Contributor

Just curious - what are the issues with updating at present?

@psychedelicious
Copy link
Contributor Author

@lillekemiker
I've only given FastAPI a casual try once and it had some mysterious errors that I wasn't sure how to even start on. When we first released 3.0, we were already behind on FastAPI due to some issue, can't remember what.

@psychedelicious
Copy link
Contributor Author

This is considerably simpler in pydantic v2:

from typing import Annotated, Literal, Union
from pydantic import BaseModel, Field, ValidationError
from pydantic.fields import FieldInfo

class Type1(BaseModel):
    type: Literal["type1"] = "type1"

class Type2(BaseModel):
    type: Literal["type2"] = "type2"

class Type3(BaseModel):
    type: Literal["type3"] = "type3"

SomeUnion = Union[Type1, Type2]

class Model(BaseModel):
    a: Annotated[SomeUnion, Field(discriminator="type")] = Field(description="test dynamic union")
    b: str = Field(description="string")

    @classmethod
    def update_union(cls):
        SomeUnion = Union[Type1,Type2,Type3]
        cls.model_fields['a'] = FieldInfo.from_annotated_attribute(Annotated[SomeUnion, Field(discriminator="type")], Field(description="test dynamic union"))
        cls.model_rebuild(force=True)

Model(a=Type1(), b="test")

try:
    Model(a=Type3(), b="test")
except ValidationError:
    print("Invalid :(")

Model.update_union()
print("Union updated")

Model(a=Type3(), b="test")
print("Valid :)")

With the improvements made in #4758 to registration of invocations, this shouldn't be a blocker.

Due to circular dependencies, I don't think we can automatically call this method during registration - it needs to be called outside it, so somewhere in api_app.py and in the jurrigged handler.

@Millu
Copy link
Contributor

Millu commented Nov 3, 2023

@psychedelicious do you think we could get this in for 3.4 with the pydantic update out of the way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants