bolkedebruin commented on PR #29721:
URL: https://github.com/apache/airflow/pull/29721#issuecomment-1465915157

   Okay, I have given this some thought. The challenge is that loading certain 
modules is expensive. Now your current approach is to hardcode any so called 
`AIRFLOW_SERIALIZERS` within the `serde` module. This creates a maintenance 
burden as the implementation of (de)serialization happens within the 
implementation module - which could lead to sync issues: this already happened 
when you encountered that I didn't implement deserialization for a kubernetes 
pod yet.
   
   The current implementation of the `serde` module already allows for string 
lookups. So I think it is better to do the lazy loading inside the place where 
the serialization happens. For example the `kubernetes` serialization could 
just register a string of `kubernetes.1.pod` (not the actual name) and load the 
module when actual (de)serialization happens. There error when trying to 
deserialize when the underlying module (e.g. kubernetes) isn't there is 
slightly different but I do not think that is a problem.
   
   In other words I think we can decentralize the lazy loading mechanism and no 
change is required to the serde module. This had the benefits of mixing eager 
loading with lazy loading, improved maintenance and better extensibility and 
less code changes.
   
   


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