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 for each variable it has. We have many variables
across many environments with a large number of DAGs in each 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. I imagine this would be a concern of any big
enterprise wanting to use Vault with Airflow
----------------------------------------------------------------
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]
