AlessioM commented on a change in pull request #13536:
URL: https://github.com/apache/airflow/pull/13536#discussion_r555000780
##########
File path: tests/providers/docker/operators/test_docker.py
##########
@@ -244,7 +244,7 @@ def test_execute_xcom_behavior(self):
xcom_push_result = xcom_push_operator.execute(None)
no_xcom_push_result = no_xcom_push_operator.execute(None)
- self.assertEqual(xcom_push_result, b'container log')
+ self.assertEqual(xcom_push_result, 'container log')
Review comment:
also: I don't think the `... else line.encode('utf-8')` is correct,
line is the last element returned from the docker client attach call,
according to the airflow documentation xcom should contain the last line if
xcom_all is False, but I don't see this happening
see:
https://github.com/docker/docker-py/blob/6da140e26c1cbe8e362b328b1c78d9c736f3a1a2/docker/api/container.py#L17
##########
File path: tests/providers/docker/operators/test_docker.py
##########
@@ -244,7 +244,7 @@ def test_execute_xcom_behavior(self):
xcom_push_result = xcom_push_operator.execute(None)
no_xcom_push_result = no_xcom_push_operator.execute(None)
- self.assertEqual(xcom_push_result, b'container log')
+ self.assertEqual(xcom_push_result, 'container log')
Review comment:
ok, yes
if we want to keep xcom results as byte strings, then we have to decode the
byte string right before logging it to json, here:
https://github.com/apache/airflow/blob/master/airflow/models/xcom.py#L235
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
