Skip to content

Bump FAB to 4.1.4#26393

Merged
ashb merged 1 commit intoapache:mainfrom
gmsantos:fab-4-1-4
Sep 15, 2022
Merged

Bump FAB to 4.1.4#26393
ashb merged 1 commit intoapache:mainfrom
gmsantos:fab-4-1-4

Conversation

@gmsantos
Copy link
Contributor

No changes that should be replicated in airflow/www/fab_security was found.

See dpgaspar/Flask-AppBuilder@v4.1.3...v4.1.4

I expect that the constraints dependencies got updated to:

flask-wtf==1.0.1
wtforms==3.0.1


^ 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 newsfragments.

@potiuk
Copy link
Member

potiuk commented Sep 14, 2022

Rebased to account for git-rev main failure :)

@potiuk
Copy link
Member

potiuk commented Sep 14, 2022

This is actually Cool. We will test if the fix with bumping "fixed" requirements that I implemented - works. Once we merge it, new PRs should take a bit longer to build image (but they should succeed) until constraints get refreshed. Then they should come back to the usual speed.

@gmsantos
Copy link
Contributor Author

Nice workflow @potiuk ! 🤞

@potiuk
Copy link
Member

potiuk commented Sep 14, 2022

Nice workflow @potiuk ! 🤞

When we see it works, yes. I am always sceptical about those kind of changes in CI, because the only way to test them is to try them live :). So I will say it's nice, when we get confirmation that it works :).

@gmsantos
Copy link
Contributor Author

Unfortunately, this PR didn't pass the CI step check for a canary run:

https://github.com/apache/airflow/actions/runs/3054848681/jobs/4927231877#step:8:616

pr-labels = []
target-repo = apache/airflow
head-repo = gmsantos/airflow
pr-number = 26393
event-name = pull_request
runs-on = ubuntu-20.04
in-workflow-build = false
build-job-description = Skip Build (look in pull_request_target)
canary-run = false
run-coverage = false

I think it's because I opened this PR from my fork, and it makes sense not to try to update the constraints from a PR outside the airflow main repo :)

@potiuk
Copy link
Member

potiuk commented Sep 14, 2022

Unfortunately, this PR didn't pass the CI step check for a canary run:

Told ya :). We are both like surgeons operating an open-heart.

Fix is coming. I will make a PR that should handle it and I will ask you to rebase it.

@potiuk
Copy link
Member

potiuk commented Sep 14, 2022

Fix in #26407

@potiuk
Copy link
Member

potiuk commented Sep 14, 2022

(alledged fix that is - I tested it locally and it worked to create the k8s venv ;) - but you never know in CI).

No changes that should be replicated in `airflow/www/fab_security`
was found.

See dpgaspar/Flask-AppBuilder@v4.1.3...v4.1.4

I expect that the constraints dependencies got updated to:

flask-wtf==1.0.1
wtforms==3.0.1
@potiuk
Copy link
Member

potiuk commented Sep 15, 2022

Rebased it after merging the fix. 🤞 @gmsantos

@ashb
Copy link
Member

ashb commented Sep 15, 2022

Targeting 2.4.1 as 2.4.0 is in feature freeze and due out today (and I don't want to test against a moving target)

@ashb ashb merged commit da06ff0 into apache:main Sep 15, 2022
@gmsantos gmsantos deleted the fab-4-1-4 branch September 15, 2022 12:56
@gmsantos
Copy link
Contributor Author

@ashb just a reminder to add this PR to the 2.4.1 milestone and to not lose it in the changelogs :)

@potiuk potiuk added this to the Airflow 2.4.1 milestone Sep 15, 2022
@potiuk
Copy link
Member

potiuk commented Sep 15, 2022

@ashb just a reminder to add this PR to the 2.4.1 milestone and to not lose it in the changelogs :)

Good point. Added it.

BTw. So far so good @gmsantos ... Looks like generaly the CI builds to push cache are failing in main when constraints are used - precisely because of the conflict between FAB 4.1.3 in constraints and 4.1.4 in setup.py (this is expected):

https://github.com/apache/airflow/actions/runs/3060594649/jobs/4939314136#step:8:564

  #53 8.655 The conflict is caused by:
  #53 8.655     apache-airflow[devel-ci] 2.4.0b1 depends on flask-appbuilder==4.1.4
  #53 8.655     The user requested (constraint) flask-appbuilder==4.1.3
  #53 8.655 
  #53 8.655 To fix this you could try to:
  #53 8.655 1. loosen the range of package versions you've specified
  #53 8.655 2. remove package versions to allow pip attempt to solve the dependency conflict
  #53 8.655 

The constraints are not yet updated (that needs a successful canarry build).

Now let's see if regular PRs will succeed regardless (they should :))

@potiuk
Copy link
Member

potiuk commented Sep 15, 2022

I want to see the system self-regulate itself :)

@potiuk
Copy link
Member

potiuk commented Sep 15, 2022

Testing it on my own PR that I just rebased: #26341

@potiuk
Copy link
Member

potiuk commented Sep 15, 2022

Looks good - the first attempt to build images failed: https://github.com/apache/airflow/actions/runs/3060753610/jobs/4940164674#step:13:185

Now it attempts to fall-back to no-constraints mode.

@ashb
Copy link
Member

ashb commented Sep 15, 2022

@potiuk The providers steps of this PR failed with version issue https://github.com/apache/airflow/actions/runs/3060640382/jobs/4939684138

@potiuk
Copy link
Member

potiuk commented Sep 15, 2022

Yep. There is one more case to handle then.

@potiuk
Copy link
Member

potiuk commented Sep 15, 2022

Fix here: #26420

@potiuk
Copy link
Member

potiuk commented Sep 15, 2022

Seems it works. So I'd cautiously say ... it works :)

@potiuk
Copy link
Member

potiuk commented Sep 15, 2022

#26341 running 🤞

@potiuk
Copy link
Member

potiuk commented Sep 16, 2022

All loks good @gmsantos - seems the last teething problems solved and we should be able to upgrade pinned dependencies without any problems in the future.

@gmsantos
Copy link
Contributor Author

Awesome! Thank you for all the hard work @potiuk !

@jedcunningham jedcunningham removed this from the Airflow 2.4.1 milestone Sep 26, 2022
@jedcunningham jedcunningham added this to the Airflow 2.4.2 milestone Sep 26, 2022
@jedcunningham
Copy link
Member

(Kicking this to 2.4.2 for now until I decide if we should range FAB in this situation or wait for 2.5.0)

@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Oct 18, 2022
ephraimbuddy pushed a commit that referenced this pull request Oct 18, 2022
No changes that should be replicated in `airflow/www/fab_security`
was found.

See dpgaspar/Flask-AppBuilder@v4.1.3...v4.1.4

I expect that the constraints dependencies got updated to:

flask-wtf==1.0.1
wtforms==3.0.1

(cherry picked from commit da06ff0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

5 participants