jason810496 commented on code in PR #58905:
URL: https://github.com/apache/airflow/pull/58905#discussion_r2580315430
##########
airflow-core/src/airflow/models/variable.py:
##########
@@ -456,25 +473,28 @@ def check_for_write_conflict(key: str) -> None:
return None
@staticmethod
- def get_variable_from_secrets(key: str) -> str | None:
+ def get_variable_from_secrets(key: str, team_id: str | None = None) -> str
| None:
"""
Get Airflow Variable by iterating over all Secret Backends.
:param key: Variable Key
+ :param team_id: ID of the team associated to the task trying to access
the variable (if any)
:return: Variable Value
"""
- # check cache first
- # enabled only if SecretCache.init() has been called first
- try:
- return SecretCache.get_variable(key)
- except SecretCache.NotPresentException:
- pass # continue business
+ # Disable cache if the variable belongs to a team. We might enable it
later
+ if not team_id:
Review Comment:
Or would it be better to support `team_id` kwargs in `SecretCache` as well?
##########
airflow-core/src/airflow/secrets/environment_variables.py:
##########
@@ -33,11 +33,16 @@ class EnvironmentVariablesBackend(BaseSecretsBackend):
def get_conn_value(self, conn_id: str) -> str | None:
return os.environ.get(CONN_ENV_PREFIX + conn_id.upper())
- def get_variable(self, key: str) -> str | None:
+ def get_variable(self, key: str, team_id: str | None = None) -> str | None:
"""
Get Airflow Variable from Environment Variable.
:param key: Variable Key
+ :param team_id: ID of the team associated to the task trying to access
the variable (if any)
:return: Variable Value
"""
+ if team_id and (team_var :=
os.environ.get(f"{VAR_ENV_PREFIX}_{team_id.upper()}__" + key.upper())):
+ # Format to set a team specific variable:
AIRFLOW_VAR__<TEAM_ID>__<VAR_KEY>
Review Comment:
> This is really not convenient for the user to use the team ID in the env
variable.
Agree on this point of view, `AIRFLOW_VAR__<TEAM_NAME>__<VAR_KEY>` will
indeed be more straightforward for the user. It _seems_ possible to achieve
even we don't change the PK, if we pass both team_name (we need extra query to
retrieve team_name from team_id for some case) and team_id from model level to
secret class level.
> Maybe, at the end, we should drop the team ID from the DB and use the name
as PK?
It might be worthwhile to raise the topic on dev list for discussion, I'm +0
with replacing teamID with just teamName as there are both pros and cons for
(teamID+teamName) or (teamName).
##########
airflow-core/src/airflow/models/variable.py:
##########
@@ -137,13 +137,15 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ team_id: str | None = None,
Review Comment:
Do we need to add `team_id` to `set` and `update` methods as well?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
