Re: [PR] Enforce supervisor schema class name matches its `type` literal [airflow]
jason810496 commented on PR #66899:
URL: https://github.com/apache/airflow/pull/66899#issuecomment-4456837127
Tried it. Runtime-binding changes the schema and also breaks the tagged
union itself, because Pydantic v2 needs the discriminator field to be a
`Literal`.
```python
from typing import Annotated, Literal, Union
from pydantic import BaseModel, Field, TypeAdapter
# --- Current pattern: Literal type field ---
class A1(BaseModel):
type: Literal["A1"] = "A1"
class A2(BaseModel):
type: Literal["A2"] = "A2"
print(A1.model_json_schema())
#. {'properties': {'type': {'const': 'A1', 'default': 'A1', 'title': 'Type',
'type': 'string'}}, 'title': 'A1', 'type': 'object'}
TypeAdapter(Annotated[Union[A1, A2], Field(discriminator="type")]) # OK
# --- Proposed pattern: runtime-bind in __init__ ---
class B(BaseModel):
type: str = "__undefined__"
def __init__(self, **kw):
super().__init__(**kw)
self.type = type(self).__name__
class B1(B): pass
class B2(B): pass
print(B1.model_json_schema())
# {'properties': {'type': {'default': '__undefined__', 'title': 'Type',
'type': 'string'}}, 'title': 'B1', 'type': 'object'}
print(B1().type) # 'B1' (instance state only; not reflected in schema)
TypeAdapter(Annotated[Union[B1, B2], Field(discriminator="type")])
# PydanticUserError: Model 'B1' needs field 'type' to be of type `Literal`
```
So `CommsDecoder`'s `TypeAdapter`-based routing relies on the `Literal`
annotation, and any downstream schema consumer relies on the `const` it
produces.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Enforce supervisor schema class name matches its `type` literal [airflow]
jason810496 commented on PR #66899: URL: https://github.com/apache/airflow/pull/66899#issuecomment-4456555941 > Not sure if considered but besides making it static check at coding time, have we considered making this a runtime property handled by Pydantic? I will follow-up on this one. BTW, not sure if there's any reason to explictly specify the `typ` literal field (e.g. binding the field at runtime might change the generated schema?) I will check it out. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Enforce supervisor schema class name matches its `type` literal [airflow]
jason810496 merged PR #66899: URL: https://github.com/apache/airflow/pull/66899 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Enforce supervisor schema class name matches its `type` literal [airflow]
jscheffl commented on PR #66899: URL: https://github.com/apache/airflow/pull/66899#issuecomment-4449419221 Not sure if considered but besides making it static check at coding time, have we considered making this a runtime property handled by Pydantic? ``` class BaseCommClass(BaseModel): """Base type class.""" type: str = "__undefined__" def __init__(self): super().__init__() self.type = self.__class__.__name__ class MyType1(BaseCommClass): """Another type.""" pass class MyType2(BaseCommClass): """Another type.""" pass print(MyType1().type) print(MyType2().type) ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Enforce supervisor schema class name matches its `type` literal [airflow]
jason810496 commented on code in PR #66899: URL: https://github.com/apache/airflow/pull/66899#discussion_r3239568120 ## task-sdk/src/airflow/sdk/execution_time/comms.py: ## @@ -869,7 +869,7 @@ class GetXComCount(BaseModel): dag_id: str run_id: str task_id: str -type: Literal["GetNumberXComs"] = "GetNumberXComs" +type: Literal["GetXComCount"] = "GetXComCount" Review Comment: Yes, I just double check that the current Go-SDK go with the Edge Worker API direction. Only the upcoming Coordinator interface will be impacted (but we haven't released any coordinator yet, so we are good!) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
