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

Bump FAB to 4.1.4 #26393

Merged
merged 1 commit into from Sep 15, 2022
Merged

Bump FAB to 4.1.4 #26393

merged 1 commit into from Sep 15, 2022

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.

None yet

5 participants