[GitHub] [airflow] fhoda commented on a change in pull request #11736: Vault with optional Variables

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

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:
users@infra.apache.org