bolkedebruin commented on issue #40974:
URL: https://github.com/apache/airflow/issues/40974#issuecomment-2267455048

   I'm mostly AFK - Olympics etc - so a bit brief and can't really deep dive 
into the code.
   
   I think targeting a new schema, your approach 2 is best, while for 
deserialization having detection of the old format and using the old 
deserializers for some time is smart. We do want people to be able to move from 
Airflow 2 -> 3 but I don't think we require them to entirely start from scratch.
   
   Having the updated schema be as close to serde's native output format, 
basically dicts with classnames and versioning information, requires the least 
passes so will be the fastest. It's also easier then so switch to another 
output format (say protobuf) if we would like that in the future. 
   
   It might be worthwhile to have a mix of a serde module and porting of the 
classes. Classes are *slow*, the overhead on the stack is significant. This is 
one of the reasons why serde does what it does. Another reason is that the 
deserialized versions of DAG and Operator are not the same as DAG and Operator. 
So it night be cleaner to keep them separated. Another one is that the 
"serialize" and "deserialize" methods in serde's modules have priority over the 
ones in the classes, so that allows you to keep the existing infrastructure in 
place while replacing it with serde's.
   
   Summarizing my thoughts:
   
   * Detecting for the old format (it night be there, or easy to do. afaik I 
created some foundatiions that allows serde to handle the old format to an 
extend by something like `deserialize(data, DAG)` where DAG is the class fo 
which it will try to find a deserializer for if it cannot detect the correct 
implementation)
   * Port parts of `SerializedBaseOperator` to serde as a module and move 
things to their respective classes. `Timetables` for example can be managed 
within `Timetable` itself. Which allows re-use in other places too.
   * Update the JSOn schema to follow more closely what serde does and have it 
properly versioned so forward compatibility works.
   
   For now my thoughts. Thanks for taking this on. I'll help where I can.
   
   


-- 
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]

Reply via email to