From b6cf6c3b8d4a4da03192c9b7db56d531792b52ab Mon Sep 17 00:00:00 2001
From: Sam Kagan <skagan@nrao.edu>
Date: Tue, 11 Jun 2024 17:29:29 -0600
Subject: [PATCH] Switched returns to raises for error responses in views where
a db transacation appeared to be open with some modified data
---
services/capability/capability/views/capability.py | 9 ++++++---
.../capability/views/capability_request.py | 12 ++++++++----
.../capability/views/capability_version.py | 3 ++-
3 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/services/capability/capability/views/capability.py b/services/capability/capability/views/capability.py
index 9dd94bbc4..aef1c7f90 100644
--- a/services/capability/capability/views/capability.py
+++ b/services/capability/capability/views/capability.py
@@ -160,7 +160,8 @@ def edit_capability(request: Request) -> Response:
if success:
return Response(body="Capability successfully edited!")
else:
- return HTTPExpectationFailed(detail=f"Unable to edit capability {capability_name}.")
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPExpectationFailed(detail=f"Unable to edit capability {capability_name}.")
@view_config(route_name="enable_capability", renderer="json")
@@ -184,7 +185,8 @@ def enable_capability(request: Request) -> Response:
if success:
return Response(body="Capability successfully enabled!")
else:
- return HTTPExpectationFailed(detail=f"Unable to enable capability {capability_name}.")
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPExpectationFailed(detail=f"Unable to enable capability {capability_name}.")
@view_config(route_name="disable_capability", renderer="json")
@@ -208,7 +210,8 @@ def disable_capability(request: Request) -> Response:
if success:
return Response(body="Capability successfully disabled!")
else:
- return HTTPExpectationFailed(detail=f"Unable to disable capability {capability_name}.")
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPExpectationFailed(detail=f"Unable to disable capability {capability_name}.")
@view_config(route_name="pause_capability", renderer="json")
diff --git a/services/capability/capability/views/capability_request.py b/services/capability/capability/views/capability_request.py
index dc4e7f859..30878378f 100644
--- a/services/capability/capability/views/capability_request.py
+++ b/services/capability/capability/views/capability_request.py
@@ -237,7 +237,8 @@ def close_capability_request(request: Request) -> Response:
request.capability_info.save_entity(capability_request)
except Exception as exc:
detail = f"sealing of {capability_request.id} failed: {exc}"
- return HTTPBadRequest(detail=detail)
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPBadRequest(detail=detail)
return Response(body=f"Capability request with ID {request_id} successfully closed.")
else:
@@ -277,7 +278,8 @@ def delete_capability_request(request: Request) -> Response:
f"Capability request for {capability_name} with ID {request_id} cannot be deleted;",
"it has existing executions.",
)
- return HTTPBadRequest(detail=bad_request_msg)
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPBadRequest(detail=bad_request_msg)
return Response(body=f"Capability request for {capability_name} with ID {request_id} successfully deleted.")
@@ -335,7 +337,8 @@ def assign_capability_request(request: Request) -> Response:
request.capability_info.save_entity(capability_request)
except Exception as exc:
detail = f"assignment of {user_name} as {group} to request {request_id} failed: {exc}"
- return HTTPBadRequest(detail=detail)
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPBadRequest(detail=detail)
return Response(json_body={"resp": response_body})
@@ -409,6 +412,7 @@ def change_srdp_compatibility_version_metadata(request: Request) -> Response:
r = requests.post(change_srdp_url, json=request.json_body)
if r.status_code != requests.codes.ok:
srdp_change_failed_msg = f"SRDP-Compatibility change failed: {r.status_code}"
- return HTTPInternalServerError(detail=srdp_change_failed_msg)
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPInternalServerError(detail=srdp_change_failed_msg)
return request.json_body
diff --git a/services/capability/capability/views/capability_version.py b/services/capability/capability/views/capability_version.py
index 803231f09..6348ba3f6 100644
--- a/services/capability/capability/views/capability_version.py
+++ b/services/capability/capability/views/capability_version.py
@@ -177,7 +177,8 @@ def update_internal_notes(request: Request) -> Response:
)
else:
no_versions_msg = f"Capability request with ID {capability_request_id} has no version {version_id}"
- return HTTPPreconditionFailed(no_versions_msg)
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPPreconditionFailed(no_versions_msg)
else:
not_found_msg = f"Capability request with ID {capability_request_id} not found."
return HTTPNotFound(detail=not_found_msg)
--
GitLab