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