fhoda commented on a change in pull request #11736:
URL: https://github.com/apache/airflow/pull/11736#discussion_r510181639



##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -112,7 +112,7 @@ class VaultBackend(BaseSecretsBackend, LoggingMixin):
     def __init__(  # pylint: disable=too-many-arguments
         self,
         connections_path: str = 'connections',
-        variables_path: str = 'variables',
+        variables_path: Optional[str] = None,

Review comment:
       @mik-laj I can revert that specific part and implement is slightly 
differently. Where it keeps the default value that was originally there 
`variable_path = 'variables'`, but then user must supply `null` to that 
parameter when defining `backend_kwargs`. This is actually how I have done it 
at my company, but I was trying to match what I saw generally in Airflow for 
this PR.
   
   @kaxil  It is meant to reduce API calls to Vault primarily. Vault produces 
errors for each task/dag that uses variable for each variable it has. We have 
many variables across many environments with a  large number of DAGs and prefer 
to manage them in the UI for the ease of the Developer. But want to keep 
sensitive Service Accounts and Secrets out of their hands directly, so 
Connections are put in Vault. Thus hitting the Vault API every time for 
Variables that are not going to be there is unnecessary.
   




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

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


Reply via email to