DM-51010: cm-client

- do not dump traceback in cli
- return 422 instead of 500 for unretriable client errors

Author: tcjennings

src/lsst/cmservice/cli/options.py

@@ -1,10 +1,12 @@
+import sys
 from collections.abc import Callable
 from enum import Enum, auto
 from functools import partial, wraps
 from typing import Any, cast
 
 import click
 from click.decorators import FC
+from httpx import HTTPStatusError
 
 from ..client.client import CMClient
 from ..common.enums import (
@@ -14,6 +16,7 @@
     NodeTypeEnum,
     StatusEnum,
 )
+from ..common.logging import LOGGER
 
 __all__ = [
     "cmclient",
@@ -65,6 +68,8 @@
     "yaml_file",
 ]
 
+logger = LOGGER.bind(module=__name__)
+
 
 class DictParamType(click.ParamType):
     """Represents the dictionary type of a CLI parameter.
@@ -514,7 +519,12 @@ def decorator(f: FC) -> FC:
         @wraps(f)
         def wrapper(*args: Any, **kwargs: Any) -> Any:
             kwargs["client"] = CMClient()
-            return f(*args, **kwargs)
+            try:
+                response = f(*args, **kwargs)
+                return response
+            except HTTPStatusError as e:
+                logger.error(e.response.text)
+                sys.exit(1)
 
         return cast(FC, wrapper)
 

src/lsst/cmservice/db/node.py

@@ -594,7 +594,9 @@ async def reject(
             Node being rejected
         """
         if self.status in [StatusEnum.accepted, StatusEnum.rescued]:
-            raise CMBadStateTransitionError(f"Can not reject {self} as it is in status {self.status}")
+            raise CMBadStateTransitionError(
+                f"Can not reject {self.fullname} as it is in status {self.status}"
+            )
 
         await self.update_values(session, status=StatusEnum.rejected)
         if self.level is LevelEnum.campaign:
@@ -625,7 +627,9 @@ async def accept(
             StatusEnum.reviewable,
             StatusEnum.rescuable,
         ]:
-            raise CMBadStateTransitionError(f"Can not accept {self} as it is in status {self.status}")
+            raise CMBadStateTransitionError(
+                f"Can not accept {self.fullname} as it is in status {self.status}"
+            )
 
         await self.update_values(session, status=StatusEnum.accepted)
 
@@ -657,7 +661,7 @@ async def reset(
             Node being reset
         """
         if self.status not in [StatusEnum.blocked, StatusEnum.rejected, StatusEnum.failed, StatusEnum.ready]:
-            raise CMBadStateTransitionError(f"Can not reset {self} as it is in status {self.status}")
+            raise CMBadStateTransitionError(f"Can not reset {self.fullname} as it is in status {self.status}")
 
         await self._clean_up_node(session, fake_reset=fake_reset)
         await self.update_values(session, status=StatusEnum.waiting, superseded=False)

src/lsst/cmservice/db/row.py

@@ -215,7 +215,7 @@ async def delete_row(
             pass
         elif hasattr(row, "status") and row.status not in DELETABLE_STATES:
             raise CMBadStateTransitionError(
-                f"Can not delete a row because it is in use {row} {row.status}",
+                f"Can not delete a row because it is in use {row.fullname} {row.status}",
             )
         try:
             await session.delete(row)

src/lsst/cmservice/routers/wrappers.py

@@ -19,7 +19,7 @@
 
 from .. import db, models
 from ..common.enums import StatusEnum
-from ..common.errors import CMMissingFullnameError, CMMissingIDError
+from ..common.errors import CMBadStateTransitionError, CMMissingFullnameError, CMMissingIDError
 
 logger = get_logger(__name__)
 
@@ -811,6 +811,8 @@ async def update_node_status(
         except CMMissingIDError as msg:
             logger.info(msg)
             raise HTTPException(status_code=404, detail=str(msg)) from msg
+        except CMBadStateTransitionError as e:
+            raise HTTPException(status_code=422, detail=str(e)) from e
         except Exception as msg:
             logger.error(msg, exc_info=True)
             raise HTTPException(status_code=500, detail=str(msg)) from msg
@@ -1098,9 +1100,11 @@ async def node_reject(
                 the_node = await db_class.get_row(session, row_id)
                 ret_val = await the_node.reject(session)
             return ret_val
-        except CMMissingIDError as msg:
-            logger.info(msg)
-            raise HTTPException(status_code=404, detail=str(msg)) from msg
+        except CMMissingIDError as e:
+            logger.info(e)
+            raise HTTPException(status_code=404, detail=str(e)) from e
+        except CMBadStateTransitionError as e:
+            raise HTTPException(status_code=422, detail=str(e)) from e
         except Exception as e:
             logger.error(e, exc_info=True)
             raise HTTPException(status_code=500, detail=str(e)) from e
@@ -1150,6 +1154,8 @@ async def node_accept(
         except CMMissingIDError as msg:
             logger.info(msg)
             raise HTTPException(status_code=404, detail=str(msg)) from msg
+        except CMBadStateTransitionError as e:
+            raise HTTPException(status_code=422, detail=str(e)) from e
         except Exception as e:
             logger.error(e, exc_info=True)
             raise HTTPException(status_code=500, detail=str(e)) from e
@@ -1200,6 +1206,8 @@ async def node_reset(
         except CMMissingIDError as msg:
             logger.info(msg)
             raise HTTPException(status_code=404, detail=str(msg)) from msg
+        except CMBadStateTransitionError as e:
+            raise HTTPException(status_code=422, detail=str(e)) from e
         except Exception as e:
             logger.error(e, exc_info=True)
             raise HTTPException(status_code=500, detail=str(e)) from e

src/lsst/cmservice/templates/wms_submit_sh.j2

@@ -11,7 +11,7 @@ set -e
 {#- Assuming native environment already setup #}
 {%- elif script_method == "htcondor" %}
 export LSST_VERSION="{{ lsst_version }}"
-export LSST_DISTRIB_DIR="{{ lsst_distrib_dir }}"
+export LSST_DISTRIB_DIR="{{ lsst_distrib_dir.rstrip("/") }}"
 source ${LSST_DISTRIB_DIR}/${LSST_VERSION}/loadLSST.bash
 setup lsst_distrib
 {# BLANK LINE #}

tests/routers/util_functions.py

@@ -401,7 +401,7 @@ async def check_update_methods(
     response = await client.post(
         f"{config.asgi.prefix}/{api_version}/{entry_class_name}/action/{entry.id}/accept",
     )
-    expect_failed_response(response, 500)
+    expect_failed_response(response, 422)
 
     update_status_model.status = StatusEnum.running
     response = await client.post(
@@ -450,13 +450,13 @@ async def check_update_methods(
     response = await client.post(
         f"{config.asgi.prefix}/{api_version}/{entry_class_name}/action/{entry.id}/reject",
     )
-    expect_failed_response(response, 500)
+    expect_failed_response(response, 422)
 
     response = await client.post(
         f"{config.asgi.prefix}/{api_version}/{entry_class_name}/action/{entry.id}/reset",
         content=reset_model.model_dump_json(),
     )
-    expect_failed_response(response, 500)
+    expect_failed_response(response, 422)
 
     # FIXME this delete test is meant to fail, but the application now allows
     #       deletion of objects in an "accepted" state, and downstream tests