Skip to content

Move access_denied_message webserver config to fab #50208

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
May 13, 2025
Merged

Move access_denied_message webserver config to fab #50208

merged 6 commits into from
May 13, 2025

Conversation

pierrejeambrun
Copy link
Member

@pierrejeambrun pierrejeambrun commented May 5, 2025 •
edited
Loading

Part of: #49896

This is still needed by the fab provider. Just confirming how things articulate between the provider / core dependency (provider requirements) on a minimal example, before moving on with other webserver config params.

@pierrejeambrun pierrejeambrun self-assigned this May 5, 2025
@pierrejeambrun pierrejeambrun added this to the Airflow 3.0.1 milestone May 5, 2025
pierrejeambrun
pierrejeambrun commented May 5, 2025
View reviewed changes
@pierrejeambrun pierrejeambrun force-pushed the move-access_denied_message-webserver-config branch from 2df9a68 to 34530a7 Compare May 5, 2025 15:06
@pierrejeambrun pierrejeambrun mentioned this pull request May 5, 2025
Closed
@pierrejeambrun pierrejeambrun force-pushed the move-access_denied_message-webserver-config branch from 34530a7 to e5e91ac Compare May 5, 2025 15:25
pierrejeambrun
pierrejeambrun commented May 5, 2025
View reviewed changes
@kaxil kaxil modified the milestones: Airflow 3.0.1, Airflow 3.0.2 May 6, 2025
@kaxil
Copy link
Member

kaxil commented May 6, 2025

Changing the milestone to 3.0.2 -- about to cut 3.0.1

@pierrejeambrun pierrejeambrun force-pushed the move-access_denied_message-webserver-config branch from d491f5e to 81ffc76 Compare May 6, 2025 16:36
jscheffl
jscheffl approved these changes May 11, 2025
View reviewed changes
Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code changes provider and core at the same time. How if somebody uses the old core and a new FAB? Are you sure it is not breaking?

@vincbeck
Copy link
Contributor

This code changes provider and core at the same time. How if somebody uses the old core and a new FAB? Are you sure it is not breaking?

I think Jens is right, but on the other side: if you use Airflow 3.0.2 (next version of Airflow not yet released that includes this change) and FAB 2.0.2 (version of FAB provider already released, NOT containing this change), then conf.get("webserver", "access_denied_message") will fail.

We already have in pyproject.toml:

"fab" = [
    "apache-airflow-providers-fab>=2.0.2" # Set from MIN_VERSION_OVERRIDE in update_airflow_pyproject_toml.py
]

Should we bump it up to 2.0.3 when the new version of FAB is released? It will have to be done as follow-up obviously.

@potiuk
Copy link
Member

potiuk commented May 12, 2025

Should we bump it up to 2.0.3 when the new version of FAB is released? It will have to be done as follow-up obviously.

If we really want to set min version of FAB then no - we do not have to wait- we can add >2.0.3 AND update fab provider in the same PR to be 2.0.3 . This should all work

@pierrejeambrun
Copy link
Member Author

pierrejeambrun commented May 13, 2025 •
edited
Loading

Thanks for the review guys.

I have updated dependencies:

  • the fab provider version to 2.0.3. And in fab provider, airflow core dependency is now >=3.0.2
  • in airflow core the fab provider dependency is now >=2.0.3.

This way basically fab 2.0.3 and core 3.0.2 can only work together. (or more recent).

Also updated to core 3.0.2 the deprecated_options

@pierrejeambrun pierrejeambrun mentioned this pull request May 13, 2025
Merged
potiuk added a commit to potiuk/airflow that referenced this pull request May 13, 2025
@potiuk
Update compatibility setup for Airflow 3.0 compatibility tests
4ba51e1 
* bump Airflow to 3.0.1
* remove common.messaging from the list of removed providers
* add fab to the list of removed providers in anticipation of
  the apache#50208
@potiuk potiuk mentioned this pull request May 13, 2025
Merged
potiuk added a commit to potiuk/airflow that referenced this pull request May 13, 2025
@potiuk
Update compatibility setup for Airflow 3.0 compatibility tests
1dc94ee 
* bump Airflow to 3.0.1
* remove common.messaging from the list of removed providers
* add fab to the list of removed providers in anticipation of
  the apache#50208
@potiuk
Copy link
Member

potiuk commented May 13, 2025 •
edited
Loading

The compat tests shoudl be fixed after we merge #50537

vincbeck
vincbeck reviewed May 13, 2025
View reviewed changes
kaxil pushed a commit that referenced this pull request May 13, 2025
@potiuk
Update compatibility setup for Airflow 3.0 compatibility tests (#50537)
41f351c 
* bump Airflow to 3.0.1
* remove common.messaging from the list of removed providers
* add fab to the list of removed providers in anticipation of
  the #50208
@pierrejeambrun pierrejeambrun force-pushed the move-access_denied_message-webserver-config branch from f2b9486 to 5f094b5 Compare May 13, 2025 13:29
@pierrejeambrun
Copy link
Member Author

#50537 has been merged, rebasing

@pierrejeambrun pierrejeambrun added the backport-to-v3-0-test Mark PR with this label to backport to v3-0-test branch label May 13, 2025
@pierrejeambrun pierrejeambrun merged commit a18a1df into apache:main May 13, 2025
96 checks passed
@pierrejeambrun pierrejeambrun deleted the move-access_denied_message-webserver-config branch May 13, 2025 14:40
@github-actions GitHub Actions
Copy link

Backport failed to create: v3-0-test. View the failure log Run details

Status Branch Result
❌ v3-0-test Commit Link

You can attempt to backport this manually by running:

cherry_picker a18a1df v3-0-test

This should apply the commit to the v3-0-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

pierrejeambrun added a commit to astronomer/airflow that referenced this pull request May 13, 2025
@pierrejeambrun
Move access_denied_message webserver config to fab (apache#50208)
847ad7f 
* Move access_denied_message webserver config to fab

* Fix CI

* Update to fab 2.0.3

* Change core to 3.0.2

* fab depends on apache-airflow 3.0.2

* Fix CI

(cherry picked from commit a18a1df)
@pierrejeambrun
Copy link
Member Author

Cherry picking PR here #50544

@pierrejeambrun pierrejeambrun mentioned this pull request May 13, 2025
Merged
@sunank200 sunank200 mentioned this pull request May 14, 2025
Merged
pierrejeambrun added a commit to astronomer/airflow that referenced this pull request May 14, 2025
@pierrejeambrun
Move access_denied_message webserver config to fab (apache#50208)
cafa1d2 
* Move access_denied_message webserver config to fab

* Fix CI

* Update to fab 2.0.3

* Change core to 3.0.2

* fab depends on apache-airflow 3.0.2

* Fix CI

(cherry picked from commit a18a1df)
@eladkal eladkal mentioned this pull request May 14, 2025
Closed
pierrejeambrun added a commit that referenced this pull request May 14, 2025
@pierrejeambrun @sunank200
[v3-0-test] Move access_denied_message webserver config to fab (#50208)…
b5d393c 
… (#50544)

* Move access_denied_message webserver config to fab (#50208)

* Move access_denied_message webserver config to fab

* Fix CI

* Update to fab 2.0.3

* Change core to 3.0.2

* fab depends on apache-airflow 3.0.2

* Fix CI

(cherry picked from commit a18a1df)

* Fix CI

* docs: conditionally render section‐move links in sections‐and‐options include (#50582)

* Add condition for section move in docs

* docs(fab): add Sphinx anchors for access_denied_message and expose_hostname config options

(cherry picked from commit 39f215f)

---------

Co-authored-by: Ankit Chaurasia <8670962+sunank200@users.noreply.github.com>
@pierrejeambrun pierrejeambrun mentioned this pull request May 21, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amoghrajesh amoghrajesh amoghrajesh approved these changes

@jscheffl jscheffl jscheffl approved these changes

@vincbeck vincbeck vincbeck approved these changes

@potiuk potiuk Awaiting requested review from potiuk potiuk is a code owner

@ashb ashb Awaiting requested review from ashb ashb is a code owner

@gopidesupavan gopidesupavan Awaiting requested review from gopidesupavan gopidesupavan is a code owner

@jedcunningham jedcunningham Awaiting requested review from jedcunningham jedcunningham is a code owner

Assignees

@pierrejeambrun pierrejeambrun

Labels
area:CLI area:ConfigTemplates area:dev-tools area:providers backport-to-v3-0-test Mark PR with this label to backport to v3-0-test branch provider:fab
Projects
None yet
Milestone
Airflow 3.0.2
Development

Successfully merging this pull request may close these issues.
6 participants
@pierrejeambrun @kaxil @vincbeck @potiuk @amoghrajesh @jscheffl