airflow-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] YingboWang commented on a change in pull request #3740: [AIRFLOW-2888] Remove shell=True and bash from task launch
Date Wed, 22 Aug 2018 00:44:43 GMT
YingboWang commented on a change in pull request #3740: [AIRFLOW-2888] Remove shell=True and
bash from task launch
URL: https://github.com/apache/incubator-airflow/pull/3740#discussion_r211801472
 
 

 ##########
 File path: airflow/executors/celery_executor.py
 ##########
 @@ -84,7 +84,7 @@ def execute_async(self, key, command,
         self.log.info("[celery] queuing {key} through celery, "
                       "queue={queue}".format(**locals()))
         self.tasks[key] = execute_command.apply_async(
-            args=[command], queue=queue)
+            args=command, queue=queue)
 
 Review comment:
   @bolkedebruin 
   Just found an issue for this commit. The update for function "apply_async" will crash scheduler
when using celery executor. Tested in local environment.
   Concern for update in line 87:
   **Before**: 
   >`args=[command]`  and command is a unicode type string . 
   > samples: `command = "airflow run dag323..."` and at this time `args = ["airflow run
example"]`
   >`execute_command("airflow run dag323")` asynchronously
   **After**: 
   >`args=command` and command is a list of short unicode strings.
   > samples: `command = ["airflow", "run", "dag323",...]` and now`args = ["airflow", "run",
"dag323", ...]`
   >`execute_command("airflow", "run", "dag323",...)` ** Error here**
   
   `execute_command.apply_async(args = command, queue = queue)` will pass a list of arguments
rather than one to function `execute_command` which is defined as taking only one argument.
The test in test_celery_executor.py right now only have one item in the testing `command`
list and can not cover this case.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services

Mime
View raw message