uranusjr commented on a change in pull request #17319:
URL: https://github.com/apache/airflow/pull/17319#discussion_r679636042



##########
File path: docs/apache-airflow/best-practices.rst
##########
@@ -109,8 +112,12 @@ or if you need to deserialize a json object from the 
variable :
 
     {{ var.json.<variable_name> }}
 
-An alternative option is to use environment variables in the top-level python 
code or use Environment Variables to create and manage Airflow variables. to 
manage Airflow Variables. This will avoid new connections to Airflow metadata 
DB every time Airflow parses the python file. For more information, see: 
:ref:`managing_variables`.
+For security purpose, you're recommended to use the :ref:`Secrets 
Backend<secrets_backend_configuration>`
+for any variable that contains sensitive data.
 
+An alternative option is to use environment variables in the top-level Python 
code or use environment variables to
+create and manage Airflow variables. This will avoid new connections to 
Airflow metadata DB every time
+Airflow parses the Python file. For more information, see: 
:ref:`managing_variables`.

Review comment:
       What does “use environment variables in the top-level Python code” mean 
here?

##########
File path: docs/apache-airflow/best-practices.rst
##########
@@ -91,13 +91,16 @@ Variables
 ---------
 
 You should avoid usage of Variables outside an operator's ``execute()`` method 
or Jinja templates if possible,
-as Variables create a connection to metadata DB of Airflow to fetch the value, 
which can slow down parsing and place extra load on the DB.
+as Variables create a connection to metadata DB of Airflow to fetch the value, 
which can slow down parsing and
+place extra load on the DB.
 
 Airflow parses all the DAGs in the background at a specific period.
-The default period is set using ``processor_poll_interval`` config, which is 
by default 1 second. During parsing, Airflow creates a new connection to the 
metadata DB for each DAG.
+The default period is set using ``processor_poll_interval`` config, which is 
by default 1 second.
+During parsing, Airflow creates a new connection to the metadata DB for each 
DAG.

Review comment:
       ```suggestion
   The default period is set using the ``processor_poll_interval`` config, 
which is 1 second by default.
   During parsing, Airflow creates a new connection to the metadata DB for each 
DAG.
   ```
   
   I think this reads better? It’s not related to this PR, but since we are 
modifying the line anyway, let’s improve it if we can.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to