-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Do not use core Airflow Flask related resources in FAB provider #45441
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
Conversation
providers/src/airflow/providers/fab/www/extensions/init_security.py
Dismissed
Show dismissed
Hide dismissed
7eb4744 to
3294c71
Compare
3294c71 to
f3e31ca
Compare
|
|
||
| if TYPE_CHECKING: | ||
| from airflow.www.extensions.init_appbuilder import AirflowAppBuilder | ||
| from airflow.providers.fab.www.extensions.init_appbuilder import AirflowAppBuilder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vincbeck We don't have add_permissions() method in from airflow.providers.fab.www.extensions.init_appbuilder import AirflowAppBuilder class. Causing airflow sync-perm command to fail on the main branch.
Updating actions and resources for all existing roles
webserver | Traceback (most recent call last):
webserver | File "/usr/local/bin/airflow", line 8, in <module>
webserver | sys.exit(main())
webserver | ^^^^^^
webserver | File "/usr/local/lib/python3.12/site-packages/airflow/__main__.py", line 58, in main
webserver | args.func(args)
webserver | File "/usr/local/lib/python3.12/site-packages/airflow/cli/cli_config.py", line 49, in command
webserver | return func(*args, **kwargs)
webserver | ^^^^^^^^^^^^^^^^^^^^^
webserver | File "/usr/local/lib/python3.12/site-packages/airflow/utils/cli.py", line 111, in wrapper
webserver | return f(*args, **kwargs)
webserver | ^^^^^^^^^^^^^^^^^^
webserver | File "/usr/local/lib/python3.12/site-packages/airflow/utils/providers_configuration_loader.py", line 55, in wrapped_function
webserver | return func(*args, **kwargs)
webserver | ^^^^^^^^^^^^^^^^^^^^^
webserver | File "/usr/local/lib/python3.12/site-packages/airflow/providers/fab/auth_manager/cli_commands/sync_perm_command.py", line 35, in sync_perm
webserver | appbuilder.add_permissions(update_perms=True)
webserver | ^^^^^^^^^^^^^^^^^^^^^^^^^^
webserver | AttributeError: 'AirflowAppBuilder' object has no attribute 'add_permissions'. Did you mean: '_add_permission'?
Is there an alternative method we should use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it still happening? The solution would be to copy over add_permissions from core Airflow to FAB provider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As part of AIP-79.
The Flask application in FAB provider should not use any Flask related resources from core Airflow. Otherwise, deleting the main Flask application in core Airflow when the legacy Airflow 2 is gone will be impossible.
^ 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.