[GitHub] [airflow] potiuk commented on issue #24880: XCOM push fails if a datetime object is in the return value

[GitBox <gi...@apache.org>]

potiuk commented on issue #24880:
URL: https://github.com/apache/airflow/issues/24880#issuecomment-1176543248

   I think we should treat it  very carefully. Changing the way how we store XCom is not something that we should do on global level - and certainly not something that we should enable "globally" for everything. The problem is that the way how we store XCom is not an interanl detail of Airflow, but it is also displayed in the UI and are part of the public API. 
   
   https://airflow.apache.org/docs/apache-airflow/stable/stable-rest-api-ref.html#operation/get_xcom_entry
   
   While it is not specify what the "string" is, there are likely already many users relying on certain form of it. 
   
   So any change that would affect all operator's way of XCom serialization form have to not only be displayable in the UI, but it also should produce the same form for all the current operators. So I would be really cautious about it.
   
   I think possibly a better approach would be to use AirflowJSon Serializer by the SageMaker Operator. That would be  much less intrusive way (only affecting the operator) and we can also make it a "recommended" solution to any other operator that wants to return non-serializable fields.  This way we have a good "solution" we can propose "anyone", but we do not change "everyeone's" behaviour. I think that's good enough or actually even "good". 
   
   Let me convert it into a discussion. It certainly is not a "bug" 


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org