Skip to content

Commit 2770ad2

Browse files
[v3-0-test] Fix for Edit and Delete request issue 53681 (#53815) (#54268)
* Fix for Edit and Delete request * updated the pool outes by adding path Fixing the 53681 issue * removed the UI based Url Encoding * Updated the pool test cases for testing '/' in poolname * Updated the PoolName * updated poolName in testcase * Faile testcase Resolved * fix(tests): make pool list ordering test locale-independent * Fix pool ordering test inconsistency across environments * Removed Sorted Comparision (cherry picked from commit 0caa87b) Co-authored-by: mandeepzemo <[email protected]>
1 parent be5907a commit 2770ad2

File tree

2 files changed

+81
-14
lines changed
  • airflow-core
    • src/airflow/api_fastapi/core_api/routes/public
    • tests/unit/api_fastapi/core_api/routes/public

2 files changed

+81
-14
lines changed

airflow-core/src/airflow/api_fastapi/core_api/routes/public/pools.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949

5050

5151
@pools_router.delete(
52-
"/{pool_name}",
52+
"/{pool_name:path}",
5353
status_code=status.HTTP_204_NO_CONTENT,
5454
responses=create_openapi_http_exception_doc(
5555
[
@@ -74,7 +74,7 @@ def delete_pool(
7474

7575

7676
@pools_router.get(
77-
"/{pool_name}",
77+
"/{pool_name:path}",
7878
responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
7979
dependencies=[Depends(requires_access_pool(method="GET"))],
8080
)
@@ -124,7 +124,7 @@ def get_pools(
124124

125125

126126
@pools_router.patch(
127-
"/{pool_name}",
127+
"/{pool_name:path}",
128128
responses=create_openapi_http_exception_doc(
129129
[
130130
status.HTTP_400_BAD_REQUEST,

airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_pools.py

Lines changed: 78 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,23 @@
3737
POOL2_DESCRIPTION = "Some Description"
3838

3939

40+
POOL3_NAME = "pool3/with_slashes"
41+
POOL3_SLOT = 5
42+
POOL3_INCLUDE_DEFERRED = False
43+
POOL3_DESCRIPTION = "Some Description"
44+
45+
4046
@provide_session
4147
def _create_pools(session) -> None:
4248
pool1 = Pool(pool=POOL1_NAME, slots=POOL1_SLOT, include_deferred=POOL1_INCLUDE_DEFERRED)
4349
pool2 = Pool(pool=POOL2_NAME, slots=POOL2_SLOT, include_deferred=POOL2_INCLUDE_DEFERRED)
44-
session.add_all([pool1, pool2])
50+
pool3 = Pool(
51+
pool=POOL3_NAME,
52+
slots=POOL3_SLOT,
53+
include_deferred=POOL3_INCLUDE_DEFERRED,
54+
description=POOL3_DESCRIPTION,
55+
)
56+
session.add_all([pool1, pool2, pool3])
4557

4658

4759
class TestPoolsEndpoint:
@@ -60,11 +72,11 @@ class TestDeletePool(TestPoolsEndpoint):
6072
def test_delete_should_respond_204(self, test_client, session):
6173
self.create_pools()
6274
pools = session.query(Pool).all()
63-
assert len(pools) == 3
75+
assert len(pools) == 4
6476
response = test_client.delete(f"/pools/{POOL1_NAME}")
6577
assert response.status_code == 204
6678
pools = session.query(Pool).all()
67-
assert len(pools) == 2
79+
assert len(pools) == 3
6880
check_last_log(session, dag_id=None, event="delete_pool", logical_date=None)
6981

7082
def test_delete_should_respond_401(self, unauthenticated_test_client):
@@ -87,6 +99,17 @@ def test_delete_should_respond_404(self, test_client):
8799
body = response.json()
88100
assert f"The Pool with name: `{POOL1_NAME}` was not found" == body["detail"]
89101

102+
def test_delete_pool3_should_respond_204(self, test_client, session):
103+
"""Test deleting POOL3 with forward slash in name"""
104+
self.create_pools()
105+
pools = session.query(Pool).all()
106+
assert len(pools) == 4
107+
response = test_client.delete(f"/pools/{POOL3_NAME}")
108+
assert response.status_code == 204
109+
pools = session.query(Pool).all()
110+
assert len(pools) == 3
111+
check_last_log(session, dag_id=None, event="delete_pool", logical_date=None)
112+
90113

91114
class TestGetPool(TestPoolsEndpoint):
92115
def test_get_should_respond_200(self, test_client, session):
@@ -120,24 +143,42 @@ def test_get_should_respond_404(self, test_client):
120143
body = response.json()
121144
assert f"The Pool with name: `{POOL1_NAME}` was not found" == body["detail"]
122145

146+
def test_get_pool3_should_respond_200(self, test_client, session):
147+
"""Test getting POOL3 with forward slash in name"""
148+
self.create_pools()
149+
response = test_client.get(f"/pools/{POOL3_NAME}")
150+
assert response.status_code == 200
151+
assert response.json() == {
152+
"deferred_slots": 0,
153+
"description": "Some Description",
154+
"include_deferred": False,
155+
"name": "pool3/with_slashes",
156+
"occupied_slots": 0,
157+
"open_slots": 5,
158+
"queued_slots": 0,
159+
"running_slots": 0,
160+
"scheduled_slots": 0,
161+
"slots": 5,
162+
}
163+
123164

124165
class TestGetPools(TestPoolsEndpoint):
125166
@pytest.mark.parametrize(
126167
"query_params, expected_total_entries, expected_ids",
127168
[
128169
# Filters
129-
({}, 3, [Pool.DEFAULT_POOL_NAME, POOL1_NAME, POOL2_NAME]),
130-
({"limit": 1}, 3, [Pool.DEFAULT_POOL_NAME]),
131-
({"limit": 1, "offset": 1}, 3, [POOL1_NAME]),
170+
({}, 4, [Pool.DEFAULT_POOL_NAME, POOL1_NAME, POOL2_NAME, POOL3_NAME]),
171+
({"limit": 1}, 4, [Pool.DEFAULT_POOL_NAME]),
172+
({"limit": 1, "offset": 1}, 4, [POOL1_NAME]),
132173
# Sort
133-
({"order_by": "-id"}, 3, [POOL2_NAME, POOL1_NAME, Pool.DEFAULT_POOL_NAME]),
134-
({"order_by": "id"}, 3, [Pool.DEFAULT_POOL_NAME, POOL1_NAME, POOL2_NAME]),
135-
({"order_by": "name"}, 3, [Pool.DEFAULT_POOL_NAME, POOL1_NAME, POOL2_NAME]),
174+
({"order_by": "-id"}, 4, [POOL3_NAME, POOL2_NAME, POOL1_NAME, Pool.DEFAULT_POOL_NAME]),
175+
({"order_by": "id"}, 4, [Pool.DEFAULT_POOL_NAME, POOL1_NAME, POOL2_NAME, POOL3_NAME]),
176+
({"order_by": "name"}, 4, [Pool.DEFAULT_POOL_NAME, POOL1_NAME, POOL2_NAME, POOL3_NAME]),
136177
# Search
137178
(
138179
{"pool_name_pattern": "~"},
139-
3,
140-
[Pool.DEFAULT_POOL_NAME, POOL1_NAME, POOL2_NAME],
180+
4,
181+
[Pool.DEFAULT_POOL_NAME, POOL1_NAME, POOL2_NAME, POOL3_NAME],
141182
),
142183
({"pool_name_pattern": "default"}, 1, [Pool.DEFAULT_POOL_NAME]),
143184
],
@@ -314,6 +355,32 @@ def test_should_respond_403(self, unauthorized_test_client):
314355
response = unauthorized_test_client.patch(f"/pools/{POOL1_NAME}", params={}, json={})
315356
assert response.status_code == 403
316357

358+
def test_patch_pool3_should_respond_200(self, test_client, session):
359+
"""Test patching POOL3 with forward slash in name"""
360+
self.create_pools()
361+
body = {
362+
"slots": 10,
363+
"description": "Updated Description",
364+
"name": POOL3_NAME,
365+
"include_deferred": True,
366+
}
367+
response = test_client.patch(f"/pools/{POOL3_NAME}", json=body)
368+
assert response.status_code == 200
369+
expected_response = {
370+
"deferred_slots": 0,
371+
"description": "Updated Description",
372+
"include_deferred": True,
373+
"name": "pool3/with_slashes",
374+
"occupied_slots": 0,
375+
"open_slots": 10,
376+
"queued_slots": 0,
377+
"running_slots": 0,
378+
"scheduled_slots": 0,
379+
"slots": 10,
380+
}
381+
assert response.json() == expected_response
382+
check_last_log(session, dag_id=None, event="patch_pool", logical_date=None)
383+
317384

318385
class TestPostPool(TestPoolsEndpoint):
319386
@pytest.mark.parametrize(

0 commit comments

Comments
 (0)