Upgrade to Flask Application Builder 4.3.9 #35085
Conversation
potiuk
requested review from
ryanahamilton,
ashb,
bbovenzi and
pierrejeambrun
as code owners
|
Due to a tight integration and override, that one would require a bit careful review and double-checking whether all the changes 4.3.6 -> 4.3.9 are correctly applied. |
potiuk
force-pushed
the
upgrade-to-fab-4.3.9
branch
from
90ecfc8 to
c922cd1
Compare
potiuk
force-pushed
the
upgrade-to-fab-4.3.9
branch
2 times, most recently
from
724850c to
ff961e3
Compare
This PR brings all the necessary changes to upgrade to FAB 4.3.9 from 4.3.6. It incorporates those changes: * dpgaspar/Flask-AppBuilder#2112 * dpgaspar/Flask-AppBuilder#2121 It also removes the limitation of the WTForms after compatibility has been implemented: * dpgaspar/Flask-AppBuilder#2138
potiuk
force-pushed
the
upgrade-to-fab-4.3.9
branch
from
ff961e3 to
e4f88a8
Compare
|
FYI @wolfdn Your fix is going to be integrated in 2.7.3 :-D |
|
Well. Just changed it to 2.8.0. It won't make it to 2.7.3 because we have quite heavy refactor for AIP-58 and we won't be able to cherry-pick this change cleanly. Sorry for that :( |
|
2.8.0 is also okay. We have (for our setup) no dependency. No pressure from my side. |
ephraimbuddy
added
the
type:misc/internal
| return { | ||
| "name": me.get("name", ""), | ||
| "email": me["upn"], | ||
| "email": me["email"], |
There was a problem hiding this comment.
Is this correct? After pulling latest main, this fails. Even if email is included in the scope claims it's not returned in the token, upn is included as before. (This could of course be an integration issue with the IDP on our end)
Either way, remember to include this in the release notes, because it may require updated scope in the webserver_config by the administrator.
There was a problem hiding this comment.
Well. This has been straight copied from FAB 4.3.9 - unless I made a mistake of course. We don't even analyse that we are just making sure that we have the same code as the original FAB Securiy Manager Has.
So if there is any problem with it - it should rather be raised to FAB not to us - our change is simply a direct pass-through to FAB.
However this is a very valid point - I see that last week FAB released version 4.3.10 which contains this one: dpgaspar/Flask-AppBuilder#2163 which solves precisely the problem you mention.
So ... time to upgrade to 4.3.10 it seems.
There was a problem hiding this comment.
Merged in #35991 -> will be released in 2.8.0
This PR brings all the necessary changes to upgrade to FAB 4.3.9 from 4.3.6.
It incorporates those changes:
authlibdpgaspar/Flask-AppBuilder#2112It also removes the limitation of the WTForms after compatibility has been implemented:
^ 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.rstor{issue_number}.significant.rst, in newsfragments.