Skip to content

Commit 8bca305

Browse files
committed
update after review
1 parent 74d8abc commit 8bca305

File tree

2 files changed

+9
-64
lines changed

2 files changed

+9
-64
lines changed

services/staging/rs_server_staging/main.py

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
from starlette.status import (
4545
HTTP_200_OK,
4646
HTTP_404_NOT_FOUND,
47-
HTTP_500_INTERNAL_SERVER_ERROR,
4847
HTTP_503_SERVICE_UNAVAILABLE,
4948
)
5049

@@ -288,18 +287,9 @@ async def get_job_status_endpoint(job_id: str = Path(..., title="The ID of the j
288287
"""Used to get status of processing job."""
289288
try:
290289
return app.extra["process_manager"].get_job(job_id)
291-
except JobNotFoundError:
290+
except JobNotFoundError as error:
292291
# Handle case when job_id is not found
293-
return JSONResponse(
294-
status_code=HTTP_404_NOT_FOUND,
295-
content={"title": "No Such Job", "detail": f"Job with ID {job_id} not found"},
296-
)
297-
except Exception as e: # pylint: disable=broad-exception-caught
298-
# Catch any other unexpected exceptions
299-
return JSONResponse(
300-
status_code=HTTP_500_INTERNAL_SERVER_ERROR,
301-
content={"title": "Internal Server Error", "detail": str(e)},
302-
)
292+
raise HTTPException(status_code=HTTP_404_NOT_FOUND, detail=f"Job with ID {job_id} not found") from error
303293

304294

305295
@router.get("/jobs")
@@ -318,18 +308,9 @@ async def delete_job_endpoint(job_id: str = Path(..., title="The ID of the job t
318308
try:
319309
app.extra["process_manager"].delete_job(job_id)
320310
return {"message": f"Job {job_id} deleted successfully"}
321-
except JobNotFoundError:
311+
except JobNotFoundError as error:
322312
# Handle case when job_id is not found
323-
return JSONResponse(
324-
status_code=HTTP_404_NOT_FOUND,
325-
content={"title": "No Such Job", "detail": f"Job with ID {job_id} not found"},
326-
)
327-
except Exception as e: # pylint: disable=broad-exception-caught
328-
# Catch any other unexpected exceptions
329-
return JSONResponse(
330-
status_code=HTTP_500_INTERNAL_SERVER_ERROR,
331-
content={"title": "Internal Server Error", "detail": str(e)},
332-
)
313+
raise HTTPException(status_code=HTTP_404_NOT_FOUND, detail=f"Job with ID {job_id} not found") from error
333314

334315

335316
@router.get("/jobs/{job_id}/results")
@@ -339,18 +320,9 @@ async def get_specific_job_result_endpoint(job_id: str = Path(..., title="The ID
339320
# Query the database to find the job by job_id
340321
job = app.extra["process_manager"].get_job(job_id)
341322
return JSONResponse(status_code=HTTP_200_OK, content=job["status"])
342-
except JobNotFoundError:
323+
except JobNotFoundError as error:
343324
# Handle case when job_id is not found
344-
return JSONResponse(
345-
status_code=HTTP_404_NOT_FOUND,
346-
content={"title": "No Such Job", "detail": f"Job with ID {job_id} not found"},
347-
)
348-
except Exception as e: # pylint: disable=broad-exception-caught
349-
# Catch any other unexpected exceptions
350-
return JSONResponse(
351-
status_code=HTTP_500_INTERNAL_SERVER_ERROR,
352-
content={"title": "Internal Server Error", "detail": str(e)},
353-
)
325+
raise HTTPException(status_code=HTTP_404_NOT_FOUND, detail=f"Job with ID {job_id} not found") from error
354326

355327

356328
# Configure OpenTelemetry

services/staging/tests/test_staging.py

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
from starlette.status import (
3030
HTTP_200_OK,
3131
HTTP_404_NOT_FOUND,
32-
HTTP_500_INTERNAL_SERVER_ERROR,
3332
HTTP_503_SERVICE_UNAVAILABLE,
3433
)
3534

@@ -226,12 +225,7 @@ async def test_get_jobs_endpoint(mocker, set_db_env_var, staging_client): # pyl
226225
(
227226
{"identifier": "non_existing_id"},
228227
HTTP_404_NOT_FOUND,
229-
{"title": "No Such Job", "detail": "Job with ID non_existing_id not found"},
230-
),
231-
(
232-
{"identifier": "trigger_500"},
233-
HTTP_500_INTERNAL_SERVER_ERROR,
234-
{"title": "Internal Server Error", "detail": "Unexpected error occurred"},
228+
{"message": "Job with ID non_existing_id not found"},
235229
),
236230
*[(job, HTTP_200_OK, job) for job in expected_jobs_test],
237231
],
@@ -265,17 +259,13 @@ async def test_get_job(
265259
- Asserts that the response status code is 200 and the returned job
266260
details match the expected job dictionary when the job exists.
267261
- Asserts that the response status code is 404 when the job does not exist.
268-
- Asserts that the response status code is 500 if other exception occurs.
269262
"""
270263
# Mock app.extra to ensure 'db_table' exists
271264
mock_db_table = mocker.MagicMock()
272265

273266
# Simulate JobNotFoundError for non-existing jobs (HTTP 404)
274267
if expected_status == HTTP_404_NOT_FOUND:
275268
mock_db_table.get_job.side_effect = JobNotFoundError
276-
# Simulate an unexpected exception (HTTP 500)
277-
elif expected_status == HTTP_500_INTERNAL_SERVER_ERROR:
278-
mock_db_table.get_job.side_effect = Exception("Unexpected error occurred")
279269
# Return an existing job normally (HTTP 200)
280270
else:
281271
mock_db_table.get_job.return_value = next(
@@ -300,12 +290,7 @@ async def test_get_job(
300290
(
301291
{"identifier": "non_existing_id"},
302292
HTTP_404_NOT_FOUND,
303-
{"title": "No Such Job", "detail": "Job with ID non_existing_id not found"},
304-
),
305-
(
306-
{"identifier": "trigger_500"},
307-
HTTP_500_INTERNAL_SERVER_ERROR,
308-
{"title": "Internal Server Error", "detail": "Unexpected error occurred"},
293+
{"message": "Job with ID non_existing_id not found"},
309294
),
310295
*[(job, HTTP_200_OK, job["status"]) for job in expected_jobs_test],
311296
],
@@ -339,17 +324,13 @@ async def test_get_job_result(
339324
- Asserts that the response status code is 200 and the returned job result
340325
matches the expected job status when the job exists.
341326
- Asserts that the response status code is 404 when the job does not exist.
342-
- Asserts that the response status code is 500 if other exception occurs.
343327
"""
344328
# Mock app.extra to ensure 'db_table' exists
345329
mock_db_table = mocker.MagicMock()
346330

347331
# Simulate JobNotFoundError for non-existing jobs (HTTP 404)
348332
if expected_status == HTTP_404_NOT_FOUND:
349333
mock_db_table.get_job.side_effect = JobNotFoundError
350-
# Simulate an unexpected exception (HTTP 500)
351-
elif expected_status == HTTP_500_INTERNAL_SERVER_ERROR:
352-
mock_db_table.get_job.side_effect = Exception("Unexpected error occurred")
353334
# Return an existing job normally (HTTP 200)
354335
else:
355336
mock_db_table.get_job.return_value = next(
@@ -375,12 +356,7 @@ async def test_get_job_result(
375356
(
376357
{"identifier": "non_existing_id"},
377358
HTTP_404_NOT_FOUND,
378-
{"title": "No Such Job", "detail": "Job with ID non_existing_id not found"},
379-
),
380-
(
381-
{"identifier": "trigger_500"},
382-
HTTP_500_INTERNAL_SERVER_ERROR,
383-
{"title": "Internal Server Error", "detail": "Unexpected error occurred"},
359+
{"message": "Job with ID non_existing_id not found"},
384360
),
385361
*[
386362
(job, HTTP_200_OK, {"message": f"Job {job['identifier']} deleted successfully"})
@@ -424,9 +400,6 @@ async def test_delete_job_endpoint(
424400
# Simulate JobNotFoundError for non-existing jobs (HTTP 404)
425401
if expected_status == HTTP_404_NOT_FOUND:
426402
mock_db_table.delete_job.side_effect = JobNotFoundError
427-
# Simulate an unexpected exception (HTTP 500)
428-
elif expected_status == HTTP_500_INTERNAL_SERVER_ERROR:
429-
mock_db_table.delete_job.side_effect = Exception("Unexpected error occurred")
430403
# Return an existing job normally (HTTP 200)
431404
else:
432405
mock_db_table.delete_job.return_value = next(

0 commit comments

Comments
 (0)