Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update flask-appbuilder authlib/oauth dependency #24516

Merged
merged 1 commit into from
Jun 19, 2022

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jun 17, 2022

The dependency we have for flask-appbuilder oauth authentication
(for github/google authentication) should follow the limits
that flask-appbuilder current version has. We added authlib there
but apparently FAB currently limits authlib to <= 1.0 - we should
follow fab rather than have our own dependency here.

This has been pointed out in
dpgaspar/Flask-AppBuilder#1861


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 newsfragement file, named {pr_number}.significant.rst, in newsfragments.

The dependency we have for flask-appbuilder oauth authentication
(for github/google authentication) should follow the limits
that flask-appbuilder current version has. We added authlib there
but apparently FAB currently limits authlib to <= 1.0 - we should
follow fab rather than have our own dependency here.

This has been pointed out in
dpgaspar/Flask-AppBuilder#1861
@potiuk
Copy link
Member Author

potiuk commented Jun 17, 2022

I am also going to update authlib to 1.0.0 (from 1.0.1) in our constraints and released images (I will use the new way of generating images :). so that users are unaffected - apparently 1.0.1 breaks authentication flow of FAB

Constraints rock.

@potiuk potiuk added this to the Airflow 2.3.3 milestone Jun 17, 2022
potiuk added a commit that referenced this pull request Jun 17, 2022
potiuk added a commit that referenced this pull request Jun 17, 2022
potiuk added a commit that referenced this pull request Jun 17, 2022
@potiuk potiuk mentioned this pull request Jun 17, 2022
1 task
@potiuk potiuk requested review from uranusjr and josh-fell June 19, 2022 18:18
@potiuk
Copy link
Member Author

potiuk commented Jun 19, 2022

This one will also be needed when we merge #24399, so that we keep authlib synced with FAB.

@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Jun 19, 2022
@potiuk potiuk merged commit 5674491 into apache:main Jun 19, 2022
@potiuk potiuk deleted the update-oauth-dependency branch June 19, 2022 19:28
@potiuk potiuk mentioned this pull request Jun 19, 2022
potiuk added a commit to potiuk/airflow that referenced this pull request Jun 22, 2022
Added note about reganerating images in June 2022.

The noteis about image changes after refreshing are now better
organized (around date of the change) - this should be more useful
by the users who will look why their images have been refreshed.

Related to: apache#24516
potiuk added a commit that referenced this pull request Jun 23, 2022
The note is about image changes after refreshing are now better
organized (around date of the change) - this should be more useful
by the users who will look why their images have been refreshed.

Related to: #24516
potiuk added a commit to potiuk/airflow that referenced this pull request Jun 29, 2022
The dependency we have for flask-appbuilder oauth authentication
(for github/google authentication) should follow the limits
that flask-appbuilder current version has. We added authlib there
but apparently FAB currently limits authlib to <= 1.0 - we should
follow fab rather than have our own dependency here.

This has been pointed out in
dpgaspar/Flask-AppBuilder#1861

(cherry picked from commit 5674491)
ephraimbuddy pushed a commit that referenced this pull request Jun 29, 2022
The note is about image changes after refreshing are now better
organized (around date of the change) - this should be more useful
by the users who will look why their images have been refreshed.

Related to: #24516

(cherry picked from commit 1a62829)
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full tests needed We need to run full set of tests for this PR to merge type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants