amoghrajesh commented on code in PR #51738:
URL: https://github.com/apache/airflow/pull/51738#discussion_r2162949904
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/assets.py:
##########
@@ -381,6 +383,7 @@ def materialize_asset(
run_after=run_after,
run_type=DagRunType.MANUAL,
triggered_by=DagRunTriggeredByType.REST_API,
+ triggering_user=user.get_name(),
Review Comment:
A qn for the `user.get_name` usages. This function can either return
username, email or user_id.
```
def get_name(self) -> str:
return self.username or self.email or self.user_id
```
for aws auth manager, which maybe a separate issue than this, but are we ok
exposing user id or email that way?
##########
airflow-core/src/airflow/cli/commands/asset_command.py:
##########
@@ -149,7 +151,14 @@ def asset_materialize(args, *, session: Session =
NEW_SESSION) -> None:
if next(dag_id_it, None) is not None:
raise SystemExit(f"More than one DAG materializes asset with
{select_message}.")
- dagrun = trigger_dag(dag_id=dag_id,
triggered_by=DagRunTriggeredByType.CLI, session=session)
+ try:
+ user = getuser()
+ except AirflowConfigException as e:
+ log.warning("Failed to get user name from os: %s", e)
Review Comment:
```suggestion
log.warning("Failed to get user name from os: %s, not setting the
triggering user", e)
```
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/dag_run.py:
##########
@@ -74,6 +74,7 @@ class DAGRunResponse(BaseModel):
run_type: DagRunType
state: DagRunState
triggered_by: DagRunTriggeredByType | None
+ triggering_user: str | None
Review Comment:
Agreed that triggereing_user is fine for now
##########
airflow-core/src/airflow/cli/commands/asset_command.py:
##########
@@ -149,7 +151,14 @@ def asset_materialize(args, *, session: Session =
NEW_SESSION) -> None:
if next(dag_id_it, None) is not None:
raise SystemExit(f"More than one DAG materializes asset with
{select_message}.")
- dagrun = trigger_dag(dag_id=dag_id,
triggered_by=DagRunTriggeredByType.CLI, session=session)
+ try:
+ user = getuser()
+ except AirflowConfigException as e:
+ log.warning("Failed to get user name from os: %s", e)
Review Comment:
Same in other usages too
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
