[GitHub] [airflow] potiuk commented on a diff in pull request #27316: [docs] best-practices add use variable with template example.

[GitBox <gi...@apache.org>]

potiuk commented on code in PR #27316:
URL: https://github.com/apache/airflow/pull/27316#discussion_r1006760090


##########
docs/apache-airflow/best-practices.rst:
##########
@@ -213,6 +213,30 @@ or if you need to deserialize a json object from the variable :
 
     {{ var.json.<variable_name> }}
 
+Ensure use variable with template in operator, not get it in top level code.
+
+Bad example:
+
+.. code-block:: python
+
+    from airflow.models import Variable
+
+    foo_var = Variable.get("foo")
+    bash_use_variable_bad = BashOperator(
+        task_id="bash_use_variable_bad", bash_command="echo variable foo=${foo_env}", env={"foo_env": foo_var}
+    )
+
+Good example:
+
+.. code-block:: python
+
+    bash_use_variable_good = BashOperator(
+        task_id="bash_use_variable_good",
+        bash_command="echo variable foo=${foo_env}",
+        env={"foo_env": "{{ var.value.get('foo') }}"},
+    )
+
+

Review Comment:
   Would be great to add also a bad example where variable is also (wrongly) used as parameter:
   ```
   bash_command="echo variable foo=${Variable.get('foo')}"  # DON'T DO THAT
   env={"foo_env": Variable.get('foo')}, # DON'T DO THAT
   ```
   
   And another example, where we show how you CAN use Variable.get in @task - decorated code.
   
   ```
   @task
   def my_task():
        var = Variable.get('foo')   # this is fine
        print(var)
   
   
   



##########
docs/apache-airflow/best-practices.rst:
##########
@@ -213,6 +213,30 @@ or if you need to deserialize a json object from the variable :
 
     {{ var.json.<variable_name> }}
 
+Ensure use variable with template in operator, not get it in top level code.

Review Comment:
   ```suggestion
   Make sure to use variable with template in operator, not in the top level code.
   ```



##########
docs/apache-airflow/best-practices.rst:
##########
@@ -213,6 +213,30 @@ or if you need to deserialize a json object from the variable :
 
     {{ var.json.<variable_name> }}
 
+Ensure use variable with template in operator, not get it in top level code.
+
+Bad example:
+
+.. code-block:: python
+
+    from airflow.models import Variable
+
+    foo_var = Variable.get("foo")

Review Comment:
   ```suggestion
       foo_var = Variable.get("foo")  # DON'T DO THAT
   ```



-- 
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-unsubscribe@airflow.apache.org

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