[GitHub] [airflow] turbaszek commented on pull request #9464: Fix DockerOperator xcom

2020-10-05 Thread GitBox


turbaszek commented on pull request #9464:
URL: https://github.com/apache/airflow/pull/9464#issuecomment-703861860


   I would be in favor of 2. + deprecation warning. I think we should return 
strings. @feluelle @mik-laj any opinions? 



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




[GitHub] [airflow] turbaszek commented on pull request #9464: Fix DockerOperator xcom

2020-10-04 Thread GitBox


turbaszek commented on pull request #9464:
URL: https://github.com/apache/airflow/pull/9464#issuecomment-703246220


   > @nullhack I think in all cases 
TestDockerOperator.test_execute_xcom_behavior is failing
   
   Yes:
   
   ```
    TestDockerOperator.test_execute_xcom_behavior 
_
   1052
   
   1053
   self = 
   1054
   
   1055
   def test_execute_xcom_behavior(self):
   1056
   self.client_mock.pull.return_value = [b'{"status":"pull log"}']
   1057
   
   1058
   kwargs = {
   1059
   'api_version': '1.19',
   1060
   'command': 'env',
   1061
   'environment': {'UNIT': 'TEST'},
   1062
   'private_environment': {'PRIVATE': 'MESSAGE'},
   1063
   'image': 'ubuntu:latest',
   1064
   'network_mode': 'bridge',
   1065
   'owner': 'unittest',
   1066
   'task_id': 'unittest',
   1067
   'volumes': ['/host/path:/container/path'],
   1068
   'working_dir': '/container/path',
   1069
   'shm_size': 1000,
   1070
   'host_tmp_dir': '/host/airflow',
   1071
   'container_name': 'test_container',
   1072
   'tty': True,
   1073
   }
   1074
   
   1075
   xcom_push_operator = DockerOperator(**kwargs, do_xcom_push=True)
   1076
   no_xcom_push_operator = DockerOperator(**kwargs, do_xcom_push=False)
   1077
   
   1078
   xcom_push_result = xcom_push_operator.execute(None)
   1079
   no_xcom_push_result = no_xcom_push_operator.execute(None)
   1080
   
   1081
   >   self.assertEqual(xcom_push_result, b'container log')
   1082
   E   AssertionError: 'container log' != b'container log'
   1083
   
   1084
   tests/providers/docker/operators/test_docker.py:248: AssertionError
   



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




[GitHub] [airflow] turbaszek commented on pull request #9464: Fix DockerOperator xcom

2020-10-01 Thread GitBox


turbaszek commented on pull request #9464:
URL: https://github.com/apache/airflow/pull/9464#issuecomment-702360591


   Hey @nullhack the test are failing, can you take a look?



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




[GitHub] [airflow] turbaszek commented on pull request #9464: Fix DockerOperator xcom

2020-09-13 Thread GitBox


turbaszek commented on pull request #9464:
URL: https://github.com/apache/airflow/pull/9464#issuecomment-691715460


   Hey @nullhack can you rebase, please? In the meantime we introduced black 
formatter in providers packages :) 



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