You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2024/03/20 10:01:50 UTC
[PR] Upgrade FAB to 4.4.1 [airflow]
potiuk opened a new pull request, #38319:
URL: https://github.com/apache/airflow/pull/38319
This upgrades FAB to version 4.4.1 and brings all the changes done to security manager between v4.3.11 and v4.4.1. As usual we need to manually synchronize the changes to security manager which we override in FAB provider.
This one also has the nice effect of bumping limits for a number of dependencies we have, such as Werkzeug (finally <4 rather than <3) and Flask (<3 rather than <2.3).
Still SQLAlchemy is not bumped though.
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
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 [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Upgrade FAB to 4.4.1 [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #38319:
URL: https://github.com/apache/airflow/pull/38319#issuecomment-2009546308
I checked [the diff between FAB 4.3.11 and 4.4.1](https://github.com/dpgaspar/Flask-AppBuilder/compare/release/4.3.11...release/4.4.1) and I dont think you forgot anything 👌
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Upgrade FAB to 4.4.1 [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #38319:
URL: https://github.com/apache/airflow/pull/38319#discussion_r1532063099
##########
airflow/api_connexion/endpoints/pool_endpoint.py:
##########
@@ -114,8 +114,11 @@ def patch_pool(
update_mask = [i.strip() for i in update_mask]
_patch_body = {}
try:
+ # MyPy infers a List[Optional[str]] type here but it should be a List[str]
+ # there is no way field is None here (UpdateMask is a List[str])
Review Comment:
Actually UpdateMask is type of update_mask (see few lines above) so it's good description (But I might add it to be more explicit once it passes all tests)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Upgrade FAB to 4.4.1 [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #38319:
URL: https://github.com/apache/airflow/pull/38319#discussion_r1531885570
##########
tests/www/views/test_session.py:
##########
@@ -97,7 +97,7 @@ def test_check_active_user(app, user_client):
user.active = False
resp = user_client.get("/home")
assert resp.status_code == 302
- assert "/logout" in resp.headers.get("Location")
+ assert "/login/?next=http%3A%2F%2Flocalhost%2Fhome" in resp.headers.get("Location")
Review Comment:
This is result of https://github.com/dpgaspar/Flask-AppBuilder/pull/2192 from FAB. And I think it's a better behaviour, because it will return the user back to the page they were originally in case someone re-activates them after they've been logged out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Upgrade FAB to 4.4.1 [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #38319:
URL: https://github.com/apache/airflow/pull/38319#issuecomment-2009180587
I'd really appreciate verifying if I applied all the changes between v4.3.11 and v4.4.1 that we should incorporate in our override @vincbeck especially - but also others.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Upgrade FAB to 4.4.1 [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #38319:
URL: https://github.com/apache/airflow/pull/38319
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Upgrade FAB to 4.4.1 [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #38319:
URL: https://github.com/apache/airflow/pull/38319#discussion_r1532065124
##########
scripts/in_container/install_airflow_and_providers.py:
##########
Review Comment:
Oh so it is related 👌 You can ignore my comment then
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Upgrade FAB to 4.4.1 [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #38319:
URL: https://github.com/apache/airflow/pull/38319#discussion_r1532108320
##########
scripts/in_container/install_airflow_and_providers.py:
##########
Review Comment:
SOMEWHAT related :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Upgrade FAB to 4.4.1 [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #38319:
URL: https://github.com/apache/airflow/pull/38319#discussion_r1531887253
##########
airflow/api_connexion/endpoints/pool_endpoint.py:
##########
@@ -114,7 +114,10 @@ def patch_pool(
update_mask = [i.strip() for i in update_mask]
_patch_body = {}
try:
- update_mask = [
+ # MyPy infers a List[Optional[str]] type here but it should be a List[str]
Review Comment:
Not sure why this one started to happen (mypy quirks) but the error signalled by mypy here makes no sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Upgrade FAB to 4.4.1 [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #38319:
URL: https://github.com/apache/airflow/pull/38319#discussion_r1532052755
##########
scripts/in_container/install_airflow_and_providers.py:
##########
Review Comment:
Is this change related to FAB upgrade? It seems it is not
##########
airflow/api_connexion/endpoints/pool_endpoint.py:
##########
@@ -114,8 +114,11 @@ def patch_pool(
update_mask = [i.strip() for i in update_mask]
_patch_body = {}
try:
+ # MyPy infers a List[Optional[str]] type here but it should be a List[str]
+ # there is no way field is None here (UpdateMask is a List[str])
Review Comment:
```suggestion
# there is no way the field is None here (update_mask is a List[str])
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Upgrade FAB to 4.4.1 [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #38319:
URL: https://github.com/apache/airflow/pull/38319#discussion_r1532061548
##########
scripts/in_container/install_airflow_and_providers.py:
##########
Review Comment:
well. Mypy started to complain in this PR. I think it's the usual "never know when MyPy starts complaining about something". It does not seem to be related to any other package upgraded in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org