fix(backend): raise ItemDeletedException for soft-deleted rows across CRUD, services, and tasks - #2508
Conversation
|
[Improvement] Possible missing import after telemetry refactor In
Overall, the soft-delete contract unification + tests look solid. Found 1 issue (0 critical, 1 improvement). |
71869cb to
6a8b4b6
Compare
f422a48 to
4334c9e
Compare
There was a problem hiding this comment.
Confirmed the earlier concern is addressed: crud/telemetry.py has from uuid import UUID at module scope, so get_trace_by_db_id() won’t hit a NameError.
The soft-delete contract unification (raising ItemDeletedException/410) and the “don’t swallow ItemDeletedException” fixes in handle_database_exceptions + handle_execution_error look consistent. Ship it.
|
Re-checked No further issues spotted in the updated diff. Ship it. |
…ed rows Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
…eted rows Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
…ft-deleted rows Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
…il directly Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
…st sets Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
…ation Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
…or test-set lookups Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
…cstrings Signed-off-by: Emanuele De Rossi <emanuele.derossi@rhesis.ai>
348a9c0 to
de79399
Compare
Purpose
Several single-item "get by ID" functions returned
Nonefor both "doesn't exist" and "soft-deleted", unlike the standardget_item_detailcontract that raisesItemDeletedException(-> HTTP 410 instead of 404). Surfaced from a code comment incrud/metric.pyexplaining whyget_metricskipped the standard pattern — turned out to be one of several.What Changed
get_metric,get_test_set,get_test_set_by_nano_id_or_slug,get_trace_by_db_idnow raise correctly;get_modeldeliberately kept its None-for-deleted contract (it's used elsewhere as an internal fallback resolver) but its organization filter now raises instead of silently skipping when empty; simplified several already-correct getters to callget_item_detaildirectly instead of duplicating its logicEndpointService._get_endpoint, the batch execution context's endpoint and test fetches,get_test_and_prompt, all fourGarakSyncServicelookups, the test-set association helpers inservices/test.py,get_prompts_for_test_set, all four preflight check functions, the test-generation attach-tests task, andservices.test_set.get_test_sethandle_database_exceptionsdecorator,handle_execution_error) that would otherwise turn the new exception into a bare 500update_test_set_attributesdepended on the old None-returning behavior to no-op gracefully, and would have started throwing on ordinary test updatesget_span_by_id,get_tool_by_providerTesting
Added regression tests for every fixed getter; several were verified by temporarily reverting the fix and confirming the test fails first. Full backend suite passes.