Skip to content

AIP-84 | Add UI Auth Links Endpoint#47662

Merged
pierrejeambrun merged 4 commits into
apache:mainfrom
jason810496:feature/AIP-84/add-ui-auth-endpoint
Mar 13, 2025
Merged

AIP-84 | Add UI Auth Links Endpoint#47662
pierrejeambrun merged 4 commits into
apache:mainfrom
jason810496:feature/AIP-84/add-ui-auth-endpoint

Conversation

@jason810496

@jason810496 jason810496 commented Mar 12, 2025

Copy link
Copy Markdown
Member

closes: #47457

What

Add /ui/auth/links endpoint

@boring-cyborg boring-cyborg Bot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. labels Mar 12, 2025
@jason810496 jason810496 mentioned this pull request Mar 12, 2025
Closed
1 task
@Lee-W Lee-W force-pushed the feature/AIP-84/add-ui-auth-endpoint branch from 91407b2 to 1da7c84 Compare March 12, 2025 09:16
@Lee-W

Lee-W commented Mar 12, 2025

Copy link
Copy Markdown
Member

There's a CI issue and is (probably) fixed in the main branch. rebased the branch

pierrejeambrun
pierrejeambrun reviewed Mar 12, 2025
View reviewed changes

@pierrejeambrun pierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks

Comment thread airflow/api_fastapi/core_api/routes/ui/auth.py
Comment thread tests/api_fastapi/core_api/routes/ui/test_auth.py Outdated
@jason810496 jason810496 force-pushed the feature/AIP-84/add-ui-auth-endpoint branch 3 times, most recently from e66cd3c to 917a7a8 Compare March 12, 2025 13:12
@bbovenzi

bbovenzi commented Mar 12, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

We should catch when the FAB provider isn't being used: cc @vincbeck

Screenshot 2025-03-12 at 10 34 49 AM

@vincbeck

vincbeck commented Mar 12, 2025

Copy link
Copy Markdown
Contributor

We should catch when the FAB provider isn't being used: cc @vincbeck

Screenshot 2025-03-12 at 10 34 49 AM

get_auth_manager returns the auth manager configured in the environment. So that should work regardless of the auth manager used. When do you get this error message?

@vincbeck

vincbeck commented Mar 12, 2025

Copy link
Copy Markdown
Contributor

Also, I am not saying this is wrong but do we want to expose it as API? We cannot pass it as config? Using an API would mean latency, so these additional links would be added to the menu after a small delay

@pierrejeambrun

pierrejeambrun commented Mar 12, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

I'm experiencing the same error as Brent. I suspect that the url_for method tries to use the flask current_app which is not set in our fastapi context.

@vincbeck

vincbeck commented Mar 12, 2025

Copy link
Copy Markdown
Contributor

I'm experiencing the same error as Brent. I suspect that the url_for method tries to use the flask current_app which is not set in our fastapi context.

Let me checkout the code and try

@vincbeck

vincbeck commented Mar 12, 2025

Copy link
Copy Markdown
Contributor

I'm experiencing the same error as Brent. I suspect that the url_for method tries to use the flask current_app which is not set in our fastapi context.

You're right, I missed that. I'll create a PR to fix it

@vincbeck vincbeck mentioned this pull request Mar 12, 2025
Merged
@vincbeck

vincbeck commented Mar 12, 2025

Copy link
Copy Markdown
Contributor

#47688

@jason810496

jason810496 commented Mar 13, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

#47688

Thanks ! I will rebase this PR after the fix being merged.

@Lee-W Lee-W force-pushed the feature/AIP-84/add-ui-auth-endpoint branch from 917a7a8 to d933d74 Compare March 13, 2025 10:12
@Lee-W

Lee-W commented Mar 13, 2025

Copy link
Copy Markdown
Member

merged #47688 and rebase this branch from the main branch

@Lee-W Lee-W force-pushed the feature/AIP-84/add-ui-auth-endpoint branch from e6d3194 to 423824c Compare March 13, 2025 11:29
@pierrejeambrun pierrejeambrun mentioned this pull request Mar 13, 2025
Merged
pierrejeambrun
pierrejeambrun approved these changes Mar 13, 2025
View reviewed changes

@pierrejeambrun pierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, working

@pierrejeambrun pierrejeambrun merged commit 6861661 into apache:main Mar 13, 2025
nailo2c pushed a commit to nailo2c/airflow that referenced this pull request Apr 4, 2025
@jason810496 @Lee-W @nailo2c
AIP-84 | Add UI Auth Links Endpoint (apache#47662)
d643564 
* AIP-84 | Add UI Auth Links Endpoint

* Fix test_auth

* style: fix coding style issues

* test(api_fastapi): add db_test mark

---------

Co-authored-by: Wei Lee <weilee.rx@gmail.com>
This was referenced Apr 6, 2025
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lee-W Lee-W Lee-W approved these changes

@pierrejeambrun pierrejeambrun pierrejeambrun approved these changes

@vincbeck vincbeck vincbeck approved these changes

@bbovenzi bbovenzi Awaiting requested review from bbovenzi

@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton

@jscheffl jscheffl Awaiting requested review from jscheffl

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy

Assignees

No one assigned

Labels

area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AIP-84 Add ui auth endpoint to expose FAB links from NavBar

5 participants

@jason810496 @Lee-W @bbovenzi @vincbeck @pierrejeambrun