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 2021/01/12 13:19:39 UTC

[GitHub] [airflow] AlessioM opened a new pull request #13635: Fix string encoding in DockerOperator when using xcom / json in xcom.py

AlessioM opened a new pull request #13635:
URL: https://github.com/apache/airflow/pull/13635


   closes: #13535
   
   fixes the xcom issue by decoding the byte array of the log file before submitting it to xcom
   
   replaces pull request #13536


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] AlessioM commented on pull request #13635: Fix string encoding in DockerOperator when using xcom / json in xcom.py

Posted by GitBox <gi...@apache.org>.
AlessioM commented on pull request #13635:
URL: https://github.com/apache/airflow/pull/13635#issuecomment-758651673


   this repo allows easy testing of the fix:
   https://github.com/AlessioM/airflow-xcom-issue
   
   however:
   the repo above has a dockeroperator that prints 10 lines (numbers 0 to 9, using a simple python script)
   xcom always contains all 10 lines, regardles if xcom_all is set or not
   
   I suspect that is because the DockerOperator uses docker-py's [client.attach](https://github.com/docker/docker-py/blob/6da140e26c1cbe8e362b328b1c78d9c736f3a1a2/docker/api/container.py#L17) wich does not return the logs separated by lines


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] AlessioM closed pull request #13635: Fix string encoding in DockerOperator when using xcom / json in xcom.py

Posted by GitBox <gi...@apache.org>.
AlessioM closed pull request #13635:
URL: https://github.com/apache/airflow/pull/13635


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] AlessioM closed pull request #13635: Fix string encoding in DockerOperator when using xcom / json in xcom.py

Posted by GitBox <gi...@apache.org>.
AlessioM closed pull request #13635:
URL: https://github.com/apache/airflow/pull/13635


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #13635: Fix string encoding in DockerOperator when using xcom / json in xcom.py

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13635:
URL: https://github.com/apache/airflow/pull/13635#issuecomment-758752669


   [The Workflow run](https://github.com/apache/airflow/actions/runs/480398597) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #13635: Fix string encoding in DockerOperator when using xcom / json in xcom.py

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13635:
URL: https://github.com/apache/airflow/pull/13635#issuecomment-758753182


   [The Workflow run](https://github.com/apache/airflow/actions/runs/480423513) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] kaxil commented on pull request #13635: Fix string encoding in DockerOperator when using xcom / json in xcom.py

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #13635:
URL: https://github.com/apache/airflow/pull/13635#issuecomment-764155414


   > I'm not sure about this one. In my opinion the default XCom supports only JSON-serializable types. And bytes are not the type. It's up to an operator to take care of that. If users want to change behaviour of XCom backend they can either use pickle (not recommended for security reasons) or create their own XCom backend:
   > https://airflow.apache.org/docs/apache-airflow/stable/concepts.html?highlight=z#custom-xcom-backend
   
   Agree with @turbaszek , the change in https://github.com/apache/airflow/pull/13536/files is actually what we need


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] AlessioM commented on pull request #13635: Fix string encoding in DockerOperator when using xcom / json in xcom.py

Posted by GitBox <gi...@apache.org>.
AlessioM commented on pull request #13635:
URL: https://github.com/apache/airflow/pull/13635#issuecomment-764534816


   ok, will close this one in favor of #13536
   thanks for the help!


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] AlessioM commented on pull request #13635: Fix string encoding in DockerOperator when using xcom / json in xcom.py

Posted by GitBox <gi...@apache.org>.
AlessioM commented on pull request #13635:
URL: https://github.com/apache/airflow/pull/13635#issuecomment-764534816


   ok, will close this one in favor of #13536
   thanks for the help!


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] kaxil commented on pull request #13635: Fix string encoding in DockerOperator when using xcom / json in xcom.py

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #13635:
URL: https://github.com/apache/airflow/pull/13635#issuecomment-764155414


   > I'm not sure about this one. In my opinion the default XCom supports only JSON-serializable types. And bytes are not the type. It's up to an operator to take care of that. If users want to change behaviour of XCom backend they can either use pickle (not recommended for security reasons) or create their own XCom backend:
   > https://airflow.apache.org/docs/apache-airflow/stable/concepts.html?highlight=z#custom-xcom-backend
   
   Agree with @turbaszek , the change in https://github.com/apache/airflow/pull/13536/files is actually what we need


----------------------------------------------------------------
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:
users@infra.apache.org