You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/08/02 12:48:17 UTC
[GitHub] [airflow] ephraimbuddy opened a new pull request, #25471: Fix Serialization error in TaskCallbackRequest
ephraimbuddy opened a new pull request, #25471:
URL: https://github.com/apache/airflow/pull/25471
How we serialize `SimpleTaskInstance `in `TaskCallbackRequest` class leads to JSON serialization error when there's start_date or end_date in the task instance. Since there's always a start_date on tis, this would always fail.
This PR aims to fix this through a new method on the SimpleTaskInstance that looks for start_date/end_date and converts them to isoformat for serialization.
--
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
[GitHub] [airflow] ashb commented on pull request #25471: Fix Serialization error in TaskCallbackRequest
Posted by GitBox <gi...@apache.org>.
ashb commented on PR #25471:
URL: https://github.com/apache/airflow/pull/25471#issuecomment-1202813944
Are we doing a 2.3.4 , or straight to 2.4.0 next?
--
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
[GitHub] [airflow] ephraimbuddy commented on a diff in pull request #25471: Fix Serialization error in TaskCallbackRequest
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on code in PR #25471:
URL: https://github.com/apache/airflow/pull/25471#discussion_r935730738
##########
airflow/callbacks/callback_requests.py:
##########
@@ -74,8 +75,8 @@ def __init__(
self.is_failure_callback = is_failure_callback
def to_json(self) -> str:
- dict_obj = self.__dict__.copy()
- dict_obj["simple_task_instance"] = dict_obj["simple_task_instance"].__dict__
+ dict_obj = copy.deepcopy(self.__dict__)
Review Comment:
I changed to `self.simple_task_instance.as_dict()` and it solved the issue I was having before that made me use deepcopy. Previously with just copy, my test was not passing as input != result.
Thanks
--
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
[GitHub] [airflow] ephraimbuddy commented on pull request #25471: Fix Serialization error in TaskCallbackRequest
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on PR #25471:
URL: https://github.com/apache/airflow/pull/25471#issuecomment-1202859730
> Are we doing a 2.3.4 , or straight to 2.4.0 next?
I think it'll be good to do 2.3.4 due to some of these bugs
cc: @jedcunningham
--
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
[GitHub] [airflow] ashb commented on a diff in pull request #25471: Fix Serialization error in TaskCallbackRequest
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #25471:
URL: https://github.com/apache/airflow/pull/25471#discussion_r935676109
##########
airflow/callbacks/callback_requests.py:
##########
@@ -74,8 +75,8 @@ def __init__(
self.is_failure_callback = is_failure_callback
def to_json(self) -> str:
- dict_obj = self.__dict__.copy()
- dict_obj["simple_task_instance"] = dict_obj["simple_task_instance"].__dict__
+ dict_obj = copy.deepcopy(self.__dict__)
Review Comment:
Was there a reason we changed this to deepcopy? It feels like it shouldn't be required if we are also calling `simple_ti.as_dict()`
--
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
[GitHub] [airflow] ephraimbuddy merged pull request #25471: Fix Serialization error in TaskCallbackRequest
Posted by GitBox <gi...@apache.org>.
ephraimbuddy merged PR #25471:
URL: https://github.com/apache/airflow/pull/25471
--
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
[GitHub] [airflow] ashb commented on a diff in pull request #25471: Fix Serialization error in TaskCallbackRequest
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #25471:
URL: https://github.com/apache/airflow/pull/25471#discussion_r935676644
##########
airflow/models/taskinstance.py:
##########
@@ -2631,6 +2631,15 @@ def __eq__(self, other):
return self.__dict__ == other.__dict__
return NotImplemented
+ def as_dict(self):
+ for key in self.__dict__:
+ if key in ['start_date', 'end_date']:
+ val = getattr(self, key)
+ if not val or isinstance(val, str):
+ continue
+ self.__dict__.update({key: val.isoformat()})
+ return self.__dict__
Review Comment:
This muteates `self` in place, which it shouldn't do. Please make it return a new dict instead.
--
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