[GitHub] [airflow] potiuk commented on a change in pull request #17319: Suggest to use secrets backend for variable when it contains sensitive data

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

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



##########
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:
       The idea here was that you could control the behaviour of top-level python code by setting "regular" environment variables and reading from those. I think we should generally discourage using Airflow Variables  at the top level of python code in Dag - (regardless if they are defined via ENV Vars or not). I think in some future version of Airflow, where we will convert access to variables into API calls, we will likely detect such access to Airflow Variables during parsing and either warn or block it. 
   
   There is very little sense to use Airflow Variables in top-level code of DAGs- even if you make "requirement" to define them via variables, it's much simpler, explicit and straightforward to simply ... use the variables via os.environment and then they should not need to follow the _VAR convention. 
   
    So I think the proposed change is not what we intended to say here.




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