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]