potiuk commented on PR #30176:
URL: https://github.com/apache/airflow/pull/30176#issuecomment-1830969268

   > @potiuk I can confirm this works for multi-line strings.
   
   My comment was rather about "why do we need method at all?" (following 
@uranusjr comment.
   
   For me the proposed syntax:
   
   ```python
   task.bash(
       """
   node $AIRFLOW_HOME/include/my_java_script.js"
   """,
       **kwargs,
   )
   ```
   
   felt a bit better than:
   
   ```python
   @task.bash(append_env=True)
   def get_ISS_coordinates():
       return """
   node $AIRFLOW_HOME/include/my_java_script.js
   """
   ```
   
   However ... I reconsidered....
   
   I actually found a very good reason why the second approach is better.
   
   It allows for much more dynamic construction of the bash command - with all 
the logic executed during task execution rather than during DAG parsing. The 
first one allows f-strings (rsolved during parsing) and JINJA (resolved during 
execution) - and pretty much nothing else.
   
   When you return string from Python funtionm you kind of combine both Python 
code wiht logic to build command nd Bash operator to actually do it - all 
during task execution. This is really powerful - especially in case of mapped 
operators for example -  much more powerful than simply passing string. 
   
   For example this one would be much more difficult to express nicely in case 
1):
   
   ```python
   @task.bash(append_env=True)
   def get_ISS_coordinates():
       base = "mypy "
       for file in get_all_possible_files_to_check():
           base += file if not file.startswith("test_")
       return base
   ```
   
   (and that's only an example).
   
   
   So - summarizing: I am all in for the proposal @josh-fell 
   


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

To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to