This is an automated email from the ASF dual-hosted git repository.
vincbeck pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 23f87019e54 Close the session after the FAB permissions collection
read (#72950)
23f87019e54 is described below
commit 23f87019e54348022932dfad723d553769ec8f6f
Author: Bingqin Wang <[email protected]>
AuthorDate: Fri Sep 11 10:24:03 2026 -0500
Close the session after the FAB permissions collection read (#72950)
---
.../fab/auth_manager/api_fastapi/services/roles.py | 55 +++++++++++-----------
.../api_fastapi/services/test_roles.py | 47 ++++++++++--------
2 files changed, 54 insertions(+), 48 deletions(-)
diff --git
a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/services/roles.py
b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/services/roles.py
index 0cb782a7d0f..6cff8e22314 100644
---
a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/services/roles.py
+++
b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/services/roles.py
@@ -195,31 +195,30 @@ class FABAuthManagerRoles:
@classmethod
def get_permissions(cls, *, order_by: str, limit: int, offset: int) ->
PermissionCollectionResponse:
- security_manager = get_fab_auth_manager().security_manager
- session = security_manager.session
- total_entries =
session.scalars(select(func.count(Permission.id))).one()
- ordering = build_ordering(
- order_by,
- allowed={
- "id": Permission.id,
- "action_id": Permission.action_id,
- "resource_id": Permission.resource_id,
- },
- )
- query = (
- select(Permission)
- .options(joinedload(Permission.action),
joinedload(Permission.resource))
- .order_by(ordering)
- .offset(offset)
- .limit(limit)
- )
- permissions = session.scalars(query).all()
- return PermissionCollectionResponse(
- permissions=[
- ActionResource(
- action=ActionModel(name=p.action.name),
resource=ResourceModel(name=p.resource.name)
- )
- for p in permissions
- ],
- total_entries=total_entries,
- )
+ with create_session(scoped=False) as session:
+ total_entries =
session.scalars(select(func.count(Permission.id))).one()
+ ordering = build_ordering(
+ order_by,
+ allowed={
+ "id": Permission.id,
+ "action_id": Permission.action_id,
+ "resource_id": Permission.resource_id,
+ },
+ )
+ query = (
+ select(Permission)
+ .options(joinedload(Permission.action),
joinedload(Permission.resource))
+ .order_by(ordering)
+ .offset(offset)
+ .limit(limit)
+ )
+ permissions = session.scalars(query).all()
+ return PermissionCollectionResponse(
+ permissions=[
+ ActionResource(
+ action=ActionModel(name=p.action.name),
resource=ResourceModel(name=p.resource.name)
+ )
+ for p in permissions
+ ],
+ total_entries=total_entries,
+ )
diff --git
a/providers/fab/tests/unit/fab/auth_manager/api_fastapi/services/test_roles.py
b/providers/fab/tests/unit/fab/auth_manager/api_fastapi/services/test_roles.py
index 7942570618d..be2d0700138 100644
---
a/providers/fab/tests/unit/fab/auth_manager/api_fastapi/services/test_roles.py
+++
b/providers/fab/tests/unit/fab/auth_manager/api_fastapi/services/test_roles.py
@@ -496,7 +496,8 @@ class TestRolesService:
FABAuthManagerRoles.patch_role(body=body, name="viewer")
assert ex.value.status_code == 404
- def test_get_permissions_success(self, get_fab_auth_manager):
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.services.roles.create_session")
+ def test_get_permissions_success(self, create_session,
get_fab_auth_manager):
session = MagicMock()
perm_obj = types.SimpleNamespace(
action=types.SimpleNamespace(name="can_read"),
@@ -506,9 +507,7 @@ class TestRolesService:
types.SimpleNamespace(one=lambda: 1),
types.SimpleNamespace(all=lambda: [perm_obj]),
]
- fab_auth_manager = MagicMock()
- fab_auth_manager.security_manager = MagicMock(session=session)
- get_fab_auth_manager.return_value = fab_auth_manager
+ create_session.return_value.__enter__.return_value = session
out = FABAuthManagerRoles.get_permissions(order_by="id", limit=10,
offset=0)
assert isinstance(out, PermissionCollectionResponse)
@@ -517,22 +516,26 @@ class TestRolesService:
assert out.permissions[0] == ActionResource(
action=Action(name="can_read"), resource=Resource(name="DAG")
)
+ create_session.assert_called_once_with(scoped=False)
+ create_session.return_value.__exit__.assert_called_once_with(None,
None, None)
- def test_get_permissions_empty(self, get_fab_auth_manager):
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.services.roles.create_session")
+ def test_get_permissions_empty(self, create_session, get_fab_auth_manager):
session = MagicMock()
session.scalars.side_effect = [
types.SimpleNamespace(one=lambda: 0),
types.SimpleNamespace(all=lambda: []),
]
- fab_auth_manager = MagicMock()
- fab_auth_manager.security_manager = MagicMock(session=session)
- get_fab_auth_manager.return_value = fab_auth_manager
+ create_session.return_value.__enter__.return_value = session
out = FABAuthManagerRoles.get_permissions(order_by="id", limit=10,
offset=0)
assert out.total_entries == 0
assert out.permissions == []
+ create_session.assert_called_once_with(scoped=False)
+ create_session.return_value.__exit__.assert_called_once_with(None,
None, None)
- def test_get_permissions_with_multiple(self, get_fab_auth_manager):
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.services.roles.create_session")
+ def test_get_permissions_with_multiple(self, create_session,
get_fab_auth_manager):
session = MagicMock()
perm_objs = [
types.SimpleNamespace(
@@ -548,9 +551,7 @@ class TestRolesService:
types.SimpleNamespace(one=lambda: 2),
types.SimpleNamespace(all=lambda: perm_objs),
]
- fab_auth_manager = MagicMock()
- fab_auth_manager.security_manager = MagicMock(session=session)
- get_fab_auth_manager.return_value = fab_auth_manager
+ create_session.return_value.__enter__.return_value = session
out = FABAuthManagerRoles.get_permissions(order_by="id", limit=10,
offset=0)
assert isinstance(out, PermissionCollectionResponse)
@@ -562,9 +563,12 @@ class TestRolesService:
assert out.permissions[1] == ActionResource(
action=Action(name="can_edit"), resource=Resource(name="DAG")
)
+ create_session.assert_called_once_with(scoped=False)
+ create_session.return_value.__exit__.assert_called_once_with(None,
None, None)
@patch("airflow.providers.fab.auth_manager.api_fastapi.services.roles.build_ordering")
- def test_get_permissions_ordering_happy_path(self, build_ordering,
get_fab_auth_manager):
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.services.roles.create_session")
+ def test_get_permissions_ordering_happy_path(self, create_session,
build_ordering, get_fab_auth_manager):
perm_obj = types.SimpleNamespace(
action=types.SimpleNamespace(name="can_read"),
resource=types.SimpleNamespace(name="DAG"),
@@ -574,9 +578,7 @@ class TestRolesService:
types.SimpleNamespace(one=lambda: 1),
types.SimpleNamespace(all=lambda: [perm_obj]),
]
- fab_auth_manager = MagicMock()
- fab_auth_manager.security_manager = MagicMock(session=session)
- get_fab_auth_manager.return_value = fab_auth_manager
+ create_session.return_value.__enter__.return_value = session
build_ordering.return_value = column("id").desc()
@@ -589,16 +591,21 @@ class TestRolesService:
args, kwargs = build_ordering.call_args
assert args[0] == "-id"
assert set(kwargs["allowed"].keys()) == {"id", "action_id",
"resource_id"}
+ create_session.assert_called_once_with(scoped=False)
+ create_session.return_value.__exit__.assert_called_once_with(None,
None, None)
@patch("airflow.providers.fab.auth_manager.api_fastapi.services.roles.build_ordering")
- def test_get_permissions_invalid_order_by_bubbles_400(self,
build_ordering, get_fab_auth_manager):
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.services.roles.create_session")
+ def test_get_permissions_invalid_order_by_bubbles_400(
+ self, create_session, build_ordering, get_fab_auth_manager
+ ):
session = MagicMock()
- fab_auth_manager = MagicMock()
- fab_auth_manager.security_manager = MagicMock(session=session)
- get_fab_auth_manager.return_value = fab_auth_manager
+ create_session.return_value.__enter__.return_value = session
build_ordering.side_effect = HTTPException(status_code=400,
detail="disallowed")
with pytest.raises(HTTPException) as ex:
FABAuthManagerRoles.get_permissions(order_by="nope", limit=10,
offset=0)
assert ex.value.status_code == 400
+ create_session.assert_called_once_with(scoped=False)
+ create_session.return_value.__exit__.assert_called_once()
