potiuk commented on code in PR #29513:
URL: https://github.com/apache/airflow/pull/29513#discussion_r1118101994
##########
airflow/models/taskinstance.py:
##########
@@ -1207,9 +1208,11 @@ def get_dagrun(self, session: Session = NEW_SESSION) ->
DagRun:
return dr
+ @staticmethod
+ @internal_api_call
@provide_session
def check_and_change_state_before_execution(
- self,
+ ti: TaskInstance,
Review Comment:
Hey everyone (mostly @mhenc @vincbeck @pierrejeambrun - but also others
@kaxil @ashb @bolkedebruin ) https://github.com/apache/airflow/pull/29776 is
the first part of the POC showing how we can slightly refactor our code for the
AIP-44 (the rest will follow if we agree that what I proposed makes sense).
I tried a few approaches how we can use the "DB" bound models in the
internal API and I thin the one that I came up with is by far simplests - it
has almost no boilerplate code, it seems to be easy to maintain and it
harnesses the power of powerful Pydantic in order to convert ORM models to
serializable classes that we will be able to send over the internal_api calls.
We discussed about using Pydantic before, to do such tasks, and after
looking at how easy it is to do the job of converting virtually any of our ORM
models to fully serializable and `validatea-ble` models, I find this approach
best.
Some of the properties:
* as little as possible code duplication - the only duplication is that we
have to define fields and their types in a separate "pydantic" class. This is a
very small overhead (those classes change rather infrequently)
* the mapping is done automatically by Pydantic, it has "orm_mode" that
deals wiht ORM mapping to Pydantic and sqlalchemy is first-class-citizen for
those
* I did not map all the relations and fields yet - only those that were easy
and likely needed, but if needs be, Pydantic cna also handle relations in
orm_mode and it can serialize linked object (for example DagRun for
TaskInstance) - it's just a matter of defining the field in Pydantic model.
* in the future we can add more validation for those objecst (also powered
by Pydantic) - for now I restricted definition of the Pydantic class fields to
use the "stdlib" types, by with Pydantic we could use more pydantic constructs
to add much more detailed validations (string lengths, min/max values etc).
This is internal API so we do not have to, but maybe we will decide it is worth
it
* pydantic is highly recommended by a lot of very serious Python projects
using it and it has bright future https://docs.pydantic.dev/blog/pydantic-v2/
with Rust implementation being many times faster.
--
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]
