Vamsi-klu commented on code in PR #73104:
URL: https://github.com/apache/airflow/pull/73104#discussion_r4002487721
##########
providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/routes/roles.py:
##########
@@ -48,7 +48,7 @@
)
def create_role(body: RoleBody) -> RoleResponse:
"""Create a new role (actions can be empty)."""
- with get_application_builder():
+ with _get_flask_app().app_context():
Review Comment:
This drops per-request sync_roles(). A role created through POST
/auth/fab/v1/roles with empty actions no longer picks up default permissions
until restart or airflow sync-perm. That is user-visible. Put it in
providers/fab/docs/changelog.rst in this PR.
##########
providers/fab/tests/unit/fab/auth_manager/api_fastapi/routes/test_users.py:
##########
@@ -219,18 +212,18 @@ def test_get_users_success_defaults(
assert resp.status_code == 200
assert resp.json() == dummy.model_dump(by_alias=True)
mock_users.get_users.assert_called_once_with(order_by="id",
limit=100, offset=0)
+ mock_get_flask_app.return_value.app_context.assert_called_once()
Review Comment:
The suite still patches _get_flask_app. The new assertions only check that
app_context() was called. Nothing proves the Flask app is created once and
reused, which is the point of the PR.
Issue two real requests and assert create_app / init_appbuilder is not
called again.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
