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 2020/10/16 03:20:36 UTC

[GitHub] [airflow] gigkokman opened a new pull request #11566: Update bash_operator's doc_string

gigkokman opened a new pull request #11566:
URL: https://github.com/apache/airflow/pull/11566


   Update doc_string warning space after bash_command to explicit inform new users


----------------------------------------------------------------
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] gigkokman commented on pull request #11566: Update bash_operator's doc_string

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


   @mik-laj would it be convenient for users to add warning about adding space after bash_command template?


----------------------------------------------------------------
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] ashb commented on a change in pull request #11566: Update bash_operator's doc_string

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11566:
URL: https://github.com/apache/airflow/pull/11566#discussion_r506552192



##########
File path: airflow/operators/bash.py
##########
@@ -72,6 +72,10 @@ class BashOperator(BaseOperator):
         users in the Web UI. Most of the default template variables are not at
         risk.
 
+        Add a space after the script name when directly calling a Bash script with the
+        ``bash_command`` argument.  This is because Airflow tries to apply a Jinja template to it,
+        which will fail.

Review comment:
       ```suggestion
           Add a space after the script name when directly calling a a ``.sh`` script with the
           ``bash_command`` argument -- for example ``bash_command="my_script.sh "``.  This
           is because Airflow tries to apply load this file and process it as a Jinja template to
           it ends with ``.sh``, which will likely not be what most users want.
   ```

##########
File path: airflow/operators/bash.py
##########
@@ -72,6 +72,10 @@ class BashOperator(BaseOperator):
         users in the Web UI. Most of the default template variables are not at
         risk.
 
+        Add a space after the script name when directly calling a Bash script with the
+        ``bash_command`` argument.  This is because Airflow tries to apply a Jinja template to it,
+        which will fail.
+

Review comment:
       You've placed this in the middle of a warning, it needs to be moved else where, perhaps above the `.. wanring::` and in to a `.. note::` block




----------------------------------------------------------------
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] ashb merged pull request #11566: Update bash_operator's doc_string

Posted by GitBox <gi...@apache.org>.
ashb merged pull request #11566:
URL: https://github.com/apache/airflow/pull/11566


   


----------------------------------------------------------------
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] ashb commented on a change in pull request #11566: Update bash_operator's doc_string

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11566:
URL: https://github.com/apache/airflow/pull/11566#discussion_r506678353



##########
File path: airflow/operators/bash.py
##########
@@ -62,6 +62,13 @@ class BashOperator(BaseOperator):
 
         bash_command = "set -e; python3 script.py '{{ next_execution_date }}'"
 
+    .. note::
+
+        Add a space after the script name when directly calling a a ``.sh`` script with the

Review comment:
       Double `a a`




----------------------------------------------------------------
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