You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "pierrejeambrun (via GitHub)" <gi...@apache.org> on 2026/07/28 09:25:13 UTC
[PR] Show user display name in the audit log via get_display_name [airflow]
pierrejeambrun opened a new pull request, #70583:
URL: https://github.com/apache/airflow/pull/70583
Follow-up to #68833.
That PR exposed `owner_display_name` in the audit log, but the action-logging decorator stored `user.get_name()` in **both** `owner` and `owner_display_name`, so the new column always matched the existing owner column — no visible benefit.
This adds a `get_display_name()` to the auth manager user model so the two can differ:
- `BaseUser.get_display_name()` is **not** abstract and defaults to `get_name()`, so auth managers written before this method existed keep working unchanged.
- The FAB user returns its full name (`get_full_name()`).
- `action_logging` now records `get_display_name()` in `owner_display_name` while `owner` keeps the raw identifier (`get_name()`), so the audit log can show a friendlier name where the auth manager provides one (and falls back to `owner` everywhere else).
The simple auth manager only knows a username, so it intentionally has no override and uses the inherited fallback.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 4.8)
Generated-by: Claude Code (Opus 4.8) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add `get_display_name` to `BaseUser` interface [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #70583:
URL: https://github.com/apache/airflow/pull/70583#discussion_r3664636493
##########
airflow-core/tests/unit/api_fastapi/logging/test_decorators.py:
##########
@@ -122,3 +126,33 @@ def test_masks_val_alias(self):
def test_value_without_key_is_still_masked(self):
result = _mask_variable_fields({"value": "secretval"})
assert result == {"value": "***"}
+
+
+class TestActionLoggingUserFields:
+ """The audit Log records the raw identifier in ``owner`` and the human-friendly
+ ``get_display_name()`` in ``owner_display_name``."""
+
+ def test_owner_and_display_name_use_the_matching_user_methods(self):
+ class FakeUser(BaseUser):
+ def get_id(self) -> str:
+ return "id"
+
+ def get_name(self) -> str:
+ return "jdoe"
+
+ def get_display_name(self) -> str:
+ return "Jane Doe"
+
+ request = MagicMock()
+ request.headers = {}
+ request.method = "GET"
+ request.query_params = {}
+ request.path_params = {}
+ request.body = AsyncMock(return_value=b"")
+ session = MagicMock()
Review Comment:
I've rewritten this path to use a real Request object instead which is cleaner and added `spec` to Session object.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add `get_display_name` to `BaseUser` interface [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #70583:
URL: https://github.com/apache/airflow/pull/70583#discussion_r3664636493
##########
airflow-core/tests/unit/api_fastapi/logging/test_decorators.py:
##########
@@ -122,3 +126,33 @@ def test_masks_val_alias(self):
def test_value_without_key_is_still_masked(self):
result = _mask_variable_fields({"value": "secretval"})
assert result == {"value": "***"}
+
+
+class TestActionLoggingUserFields:
+ """The audit Log records the raw identifier in ``owner`` and the human-friendly
+ ``get_display_name()`` in ``owner_display_name``."""
+
+ def test_owner_and_display_name_use_the_matching_user_methods(self):
+ class FakeUser(BaseUser):
+ def get_id(self) -> str:
+ return "id"
+
+ def get_name(self) -> str:
+ return "jdoe"
+
+ def get_display_name(self) -> str:
+ return "Jane Doe"
+
+ request = MagicMock()
+ request.headers = {}
+ request.method = "GET"
+ request.query_params = {}
+ request.path_params = {}
+ request.body = AsyncMock(return_value=b"")
+ session = MagicMock()
Review Comment:
I've rewritten this path to use a real Request object instead which is cleaner
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add `get_display_name` to `BaseUser` interface [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #70583:
URL: https://github.com/apache/airflow/pull/70583
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add `get_display_name` to `BaseUser` interface [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #70583:
URL: https://github.com/apache/airflow/pull/70583#discussion_r3664525929
##########
airflow-core/tests/unit/api_fastapi/logging/test_decorators.py:
##########
@@ -122,3 +126,33 @@ def test_masks_val_alias(self):
def test_value_without_key_is_still_masked(self):
result = _mask_variable_fields({"value": "secretval"})
assert result == {"value": "***"}
+
+
+class TestActionLoggingUserFields:
+ """The audit Log records the raw identifier in ``owner`` and the human-friendly
+ ``get_display_name()`` in ``owner_display_name``."""
+
+ def test_owner_and_display_name_use_the_matching_user_methods(self):
+ class FakeUser(BaseUser):
+ def get_id(self) -> str:
+ return "id"
+
+ def get_name(self) -> str:
+ return "jdoe"
+
+ def get_display_name(self) -> str:
+ return "Jane Doe"
+
+ request = MagicMock()
+ request.headers = {}
+ request.method = "GET"
+ request.query_params = {}
+ request.path_params = {}
+ request.body = AsyncMock(return_value=b"")
+ session = MagicMock()
Review Comment:
Can we add a spec or autospec please?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add `get_display_name` to `BaseUser` interface [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #70583:
URL: https://github.com/apache/airflow/pull/70583#discussion_r3664636493
##########
airflow-core/tests/unit/api_fastapi/logging/test_decorators.py:
##########
@@ -122,3 +126,33 @@ def test_masks_val_alias(self):
def test_value_without_key_is_still_masked(self):
result = _mask_variable_fields({"value": "secretval"})
assert result == {"value": "***"}
+
+
+class TestActionLoggingUserFields:
+ """The audit Log records the raw identifier in ``owner`` and the human-friendly
+ ``get_display_name()`` in ``owner_display_name``."""
+
+ def test_owner_and_display_name_use_the_matching_user_methods(self):
+ class FakeUser(BaseUser):
+ def get_id(self) -> str:
+ return "id"
+
+ def get_name(self) -> str:
+ return "jdoe"
+
+ def get_display_name(self) -> str:
+ return "Jane Doe"
+
+ request = MagicMock()
+ request.headers = {}
+ request.method = "GET"
+ request.query_params = {}
+ request.path_params = {}
+ request.body = AsyncMock(return_value=b"")
+ session = MagicMock()
Review Comment:
I've rewritten this path to use a real Request object instead which is cleaner and added `spec` to Session mock object.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add `get_display_name` to `BaseUser` interface [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #70583:
URL: https://github.com/apache/airflow/pull/70583#issuecomment-5102459322
Also adjust the front-end filter so we are now searching for 'owner_display_name' instead. This way 'Thor' will work.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org