kaxil commented on a change in pull request #18764:
URL: https://github.com/apache/airflow/pull/18764#discussion_r723213321



##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -74,22 +74,26 @@ class SecretsManagerBackend(BaseSecretsBackend, 
LoggingMixin):
     However, these lists can be extended using the configuration parameter 
``extra_conn_words``.
 
     :param connections_prefix: Specifies the prefix of the secret to read to 
get Connections.
-        If set to None (null), requests for connections will not be sent to 
AWS Secrets Manager
+        If set to None (null value in the configuration), requests for 
connections will not be
+        sent to AWS Secrets Manager. If you don't want a connections_prefix, 
set it as an empty string
     :type connections_prefix: str
     :param variables_prefix: Specifies the prefix of the secret to read to get 
Variables.
-        If set to None (null), requests for variables will not be sent to AWS 
Secrets Manager
+        If set to None (null value in the configuration), requests for 
variables will not be sent to
+        AWS Secrets Manager. If you don't want a variables_prefix, set it as 
an empty string
     :type variables_prefix: str
     :param config_prefix: Specifies the prefix of the secret to read to get 
Configurations.
-        If set to None (null), requests for configurations will not be sent to 
AWS Secrets Manager
+        If set to None (null value in the configuration), requests for 
configurations will not be sent to
+        AWS Secrets Manager. If you don't want a config_prefix, set it as an 
empty string

Review comment:
       for the following case:
   
   >In some cases organizations may not want to use Vault for Airflow Variable, 
but instead for Airflow Connections only. The proposed PR makes Variables 
optional in Vault when using Vault as the Alternative Secrets Backend.
   
   The changes here eliminate Airflow requests to Vault when the variables_path 
parameter is not defined in backend_kwargs. Thus reducing the burden on Vault 
deployments and reduced error messages when only Connection are truly 
implemented.
   
   

##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -74,22 +74,26 @@ class SecretsManagerBackend(BaseSecretsBackend, 
LoggingMixin):
     However, these lists can be extended using the configuration parameter 
``extra_conn_words``.
 
     :param connections_prefix: Specifies the prefix of the secret to read to 
get Connections.
-        If set to None (null), requests for connections will not be sent to 
AWS Secrets Manager
+        If set to None (null value in the configuration), requests for 
connections will not be
+        sent to AWS Secrets Manager. If you don't want a connections_prefix, 
set it as an empty string
     :type connections_prefix: str
     :param variables_prefix: Specifies the prefix of the secret to read to get 
Variables.
-        If set to None (null), requests for variables will not be sent to AWS 
Secrets Manager
+        If set to None (null value in the configuration), requests for 
variables will not be sent to
+        AWS Secrets Manager. If you don't want a variables_prefix, set it as 
an empty string
     :type variables_prefix: str
     :param config_prefix: Specifies the prefix of the secret to read to get 
Configurations.
-        If set to None (null), requests for configurations will not be sent to 
AWS Secrets Manager
+        If set to None (null value in the configuration), requests for 
configurations will not be sent to
+        AWS Secrets Manager. If you don't want a config_prefix, set it as an 
empty string

Review comment:
       for the following case:
   
   >In some cases organizations may not want to use Vault for Airflow Variable, 
but instead for Airflow Connections only. The proposed PR makes Variables 
optional in Vault when using Vault as the Alternative Secrets Backend.
   
   >The changes here eliminate Airflow requests to Vault when the 
variables_path parameter is not defined in backend_kwargs. Thus reducing the 
burden on Vault deployments and reduced error messages when only Connection are 
truly implemented.
   
   




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