sjyangkevin commented on code in PR #54308:
URL: https://github.com/apache/airflow/pull/54308#discussion_r2275246862
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py:
##########
@@ -119,6 +119,11 @@ def _update_hitl_detail(
"and is not allowed to write again.",
)
+ if hitl_detail_model.respondents and user.get_name() not in
hitl_detail_model.respondents:
Review Comment:
Hi @Lee-W , I attempted to see if we could refactor this code to check if a
user is a respondent using `user_id` but couldn't find a good way to implement
it. Not sure if you had any suggestion.
First, I think a straight-forward way is to query the DB here to fetch a
list of (user_id, username). Then, we can filter this list using the
`respondents`. Then, check if the user_id is in the filtered list. However,
seems like the user information is available only in `ab_user` when using
FabAuthManager. In SimpleAuthManager, a different implementation is used to
fetch the user list.
Then, I looked into if it is possible to implement the `username` to
`user_id` mapping at 1.) operator-level, or 2.) execution time API level.
1.) if it is possible to have some kind of call to get a list of users
through a unified method across different auth manager, we can implement this
mapping from `username` to `user_id` here and commit `user_id` to the DB. In
this case, the check will be easier.
2.) I think this case might also require some sorts of join with a "User"
model or a way to fetch a list of users.
--
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]
