Skip to content

Conversation

@Abhishek1559
Copy link
Contributor

@Abhishek1559 Abhishek1559 commented Aug 12, 2025

This closes Fab auth manager returns get_id of integer type where str is expected #54298
closes: #54298


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

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

need a test case for it

@jason810496 jason810496 self-requested a review August 14, 2025 06:01
@Abhishek1559
Copy link
Contributor Author

added the test case

@Abhishek1559 Abhishek1559 requested a review from Lee-W August 14, 2025 10:39
Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for the catch!

@kyungjunleeme
Copy link
Contributor

nice pr :)
hello I just curious.
I think this issue is a fairly fundamental/basic part, so why hasn’t it come to light until now?

@Lee-W
Copy link
Member

Lee-W commented Aug 18, 2025

nice pr :) hello I just curious. I think this issue is a fairly fundamental/basic part, so why hasn’t it come to light until now?

I guess str, int difference doesn't really matter in these cases 🤔

Copy link
Contributor

@vincbeck vincbeck left a comment

Choose a reason for hiding this comment

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

One nit but overall LGTM

@Lee-W Lee-W force-pushed the FAB_Auth_Manager_Error branch from b735b53 to 22cb68b Compare August 26, 2025 02:49
@Lee-W
Copy link
Member

Lee-W commented Aug 26, 2025

As AIP-90 kinda needs this, I just updated the tests. Since I modified part of the code, it would be nice if @jason810496 or @vincbeck could take another look and merge it if there's no issue :)

@Lee-W
Copy link
Member

Lee-W commented Aug 26, 2025

cc @KevinYang21

@Lee-W Lee-W changed the title To fix Fab auth manager returns get_id of integer type where str is expected #54298 To fix Fab auth manager returns get_id of integer type where str is expected Aug 26, 2025
Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@Lee-W Lee-W merged commit 08ed097 into apache:main Aug 26, 2025
72 of 73 checks passed
@boring-cyborg
Copy link

boring-cyborg bot commented Aug 26, 2025

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
…xpected (apache#54384)

Co-authored-by: Wei Lee <weilee.rx@gmail.com>
nothingmin pushed a commit to nothingmin/airflow that referenced this pull request Sep 2, 2025
…xpected (apache#54384)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fab auth manager returns get_id of integer type where str is expected

5 participants