Skip to content

Add get_display_name to BaseUser interface - #70583

Merged
pierrejeambrun merged 4 commits into
apache:mainfrom
astronomer:add-user-get-display-name
Jul 28, 2026
Merged

Add get_display_name to BaseUser interface #70583
pierrejeambrun merged 4 commits into
apache:mainfrom
astronomer:add-user-get-display-name

Conversation

@pierrejeambrun

@pierrejeambrun pierrejeambrun commented Jul 28, 2026

Copy link
Copy Markdown
Member

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.

For SimpleAuthManager, nothing changes. (display_name = name), new method isn't overriden:
Screenshot 2026-07-28 at 11 26 59

For FabAuthManager, which implements get_display_name as a get_full_name:
Screenshot 2026-07-28 at 11 33 01

Filtering
Screenshot 2026-07-28 at 11 57 15


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.8)

Generated-by: Claude Code (Opus 4.8) following the guidelines

The audit log's owner_display_name always equalled owner because the action
logger stored get_name() in both fields. Add a get_display_name() to the auth
manager user model, defaulting to get_name() so auth managers written before it
keep working unchanged, and have the FAB user return its full name. The action
logger now records get_display_name() as owner_display_name, so the audit log
can show a friendlier name than the raw owner identifier where one exists.

Follow-up to apache#68833.
@pierrejeambrun
pierrejeambrun marked this pull request as ready for review July 28, 2026 09:25
@pierrejeambrun pierrejeambrun changed the title Show user display name in the audit log via get_display_name Add get_display_name to BaseUser interface Jul 28, 2026
@pierrejeambrun

Copy link
Copy Markdown
Member Author

Also adjust the front-end filter so we are now searching for 'owner_display_name' instead. This way 'Thor' will work.

@amoghrajesh amoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one nit.

Comment thread airflow-core/tests/unit/api_fastapi/logging/test_decorators.py Outdated
The audit log user column now shows owner_display_name, so its filter should
match that value. Add server-side pattern and prefix-pattern search params on
owner_display_name and point the existing User filter at them, and drop the
redundant UI owner fallback so the displayed value and the filter stay in sync.
A real Request built from a minimal ASGI scope supplies genuine empty
headers/query/path params, so nothing needs stubbing and the test cannot drift
from the real request interface. The session stays a spec'd mock to capture the
logged row.
@pierrejeambrun
pierrejeambrun merged commit 2f5fb76 into apache:main Jul 28, 2026
156 checks passed
@pierrejeambrun
pierrejeambrun deleted the add-user-get-display-name branch July 28, 2026 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants