diff --git a/changelog.d/20240905_201903_roman_lambda_error_handling.md b/changelog.d/20240905_201903_roman_lambda_error_handling.md new file mode 100644 index 00000000000..323bf7960bf --- /dev/null +++ b/changelog.d/20240905_201903_roman_lambda_error_handling.md @@ -0,0 +1,15 @@ +### Changed + +- Lambda function endpoints now return 500 instead of 404 + if a function's metadata is invalid + () + +- An unknown lambda function type is now treated as invalid metadata + and the function is no longer included in the list endpoint output + () + +### Fixed + +- One lambda function with invalid metadata will no longer + break function listing + () diff --git a/cvat/apps/lambda_manager/tests/assets/functions.json b/cvat/apps/lambda_manager/tests/assets/functions.json index 15e9cbf1156..84da63ca714 100644 --- a/cvat/apps/lambda_manager/tests/assets/functions.json +++ b/cvat/apps/lambda_manager/tests/assets/functions.json @@ -90,95 +90,95 @@ "httpPort": 49158 } }, - "test-model-has-non-type": { + "test-model-has-state-building": { "metadata": { - "name": "test-model-has-non-type", + "name": "test-model-has-state-building", "annotations": { "framework": "openvino", - "name": "Non type", - "spec": "[\n { \"id\": 0, \"name\": \"person\" }, \n { \"id\": 1, \"name\": \"bicycle\" }, \n { \"id\": 2, \"name\": \"car\" } \n]\n" + "name": "State is building", + "spec": "[\n { \"id\": 0, \"name\": \"person\" }, \n { \"id\": 1, \"name\": \"bicycle\" }, \n { \"id\": 2, \"name\": \"car\" } \n]\n", + "type": "detector" } }, "spec": { - "description": "Test non type" + "description": "Test state building" }, "status": { - "state": "ready", - "httpPort": 49160 + "state": "building" } }, - "test-model-has-wrong-type": { + "test-model-has-state-error": { "metadata": { - "name": "test-model-has-wrong-type", + "name": "test-model-has-state-building", "annotations": { "framework": "openvino", - "name": "Non type", + "name": "State is error", "spec": "[\n { \"id\": 0, \"name\": \"person\" }, \n { \"id\": 1, \"name\": \"bicycle\" }, \n { \"id\": 2, \"name\": \"car\" } \n]\n", - "type": "car-bicycle-person-detector" + "type": "detector" } }, "spec": { - "description": "Test wrong type" + "description": "Test state error" }, "status": { - "state": "ready", - "httpPort": 49161 + "state": "error" } - }, - "test-model-has-unknown-type": { + } + }, + "negative": { + "test-model-has-non-type": { "metadata": { - "name": "test-model-has-unknown-type", + "name": "test-model-has-non-type", "annotations": { "framework": "openvino", "name": "Non type", - "spec": "[\n { \"id\": 0, \"name\": \"person\" }, \n { \"id\": 1, \"name\": \"bicycle\" }, \n { \"id\": 2, \"name\": \"car\" } \n]\n", - "type": "unknown" + "spec": "[\n { \"id\": 0, \"name\": \"person\" }, \n { \"id\": 1, \"name\": \"bicycle\" }, \n { \"id\": 2, \"name\": \"car\" } \n]\n" } }, "spec": { - "description": "Test unknown type" + "description": "Test non type" }, "status": { "state": "ready", - "httpPort": 49162 + "httpPort": 49160 } }, - "test-model-has-state-building": { + "test-model-has-wrong-type": { "metadata": { - "name": "test-model-has-state-building", + "name": "test-model-has-wrong-type", "annotations": { "framework": "openvino", - "name": "State is building", + "name": "Non type", "spec": "[\n { \"id\": 0, \"name\": \"person\" }, \n { \"id\": 1, \"name\": \"bicycle\" }, \n { \"id\": 2, \"name\": \"car\" } \n]\n", - "type": "detector" + "type": "car-bicycle-person-detector" } }, "spec": { - "description": "Test state building" + "description": "Test wrong type" }, "status": { - "state": "building" + "state": "ready", + "httpPort": 49161 } }, - "test-model-has-state-error": { + "test-model-has-unknown-type": { "metadata": { - "name": "test-model-has-state-building", + "name": "test-model-has-unknown-type", "annotations": { "framework": "openvino", - "name": "State is error", + "name": "Non type", "spec": "[\n { \"id\": 0, \"name\": \"person\" }, \n { \"id\": 1, \"name\": \"bicycle\" }, \n { \"id\": 2, \"name\": \"car\" } \n]\n", - "type": "detector" + "type": "unknown" } }, "spec": { - "description": "Test state error" + "description": "Test unknown type" }, "status": { - "state": "error" + "state": "ready", + "httpPort": 49162 } - } - }, - "negative": { + }, "test-model-has-non-unique-labels": { "metadata": { "name": "test-model-has-non-unique-labels", diff --git a/cvat/apps/lambda_manager/tests/test_lambda.py b/cvat/apps/lambda_manager/tests/test_lambda.py index e360ab1996d..c86b4eaa61a 100644 --- a/cvat/apps/lambda_manager/tests/test_lambda.py +++ b/cvat/apps/lambda_manager/tests/test_lambda.py @@ -76,7 +76,7 @@ def __exit__(self, exception_type, exception_value, traceback): class _LambdaTestCaseBase(APITestCase): def setUp(self): - self.client = APIClient() + self.client = APIClient(raise_request_exception=False) http_patcher = mock.patch('cvat.apps.lambda_manager.views.LambdaGateway._http', side_effect = self._get_data_from_lambda_manager_http) self.addCleanup(http_patcher.stop) @@ -295,16 +295,21 @@ def test_api_v2_lambda_functions_list_empty(self, mock_http): self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) - @mock.patch('cvat.apps.lambda_manager.views.LambdaGateway._http', return_value = functions["negative"]) - def test_api_v2_lambda_functions_list_wrong(self, mock_http): + @mock.patch( + 'cvat.apps.lambda_manager.views.LambdaGateway._http', + return_value={**functions["negative"], id_function_detector: functions["positive"][id_function_detector]} + ) + def test_api_v2_lambda_functions_list_negative(self, mock_http): response = self._get_request(LAMBDA_FUNCTIONS_PATH, self.admin) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.status_code, status.HTTP_200_OK) + # the positive function must remain visible + visible_ids = {f["id"] for f in response.data} + self.assertEqual(visible_ids, {id_function_detector}) def test_api_v2_lambda_functions_read(self): ids_functions = [id_function_detector, id_function_interactor,\ - id_function_tracker, id_function_reid_with_response_data, \ - id_function_non_type, id_function_wrong_type, id_function_unknown_type] + id_function_tracker, id_function_reid_with_response_data] for id_func in ids_functions: path = f'{LAMBDA_FUNCTIONS_PATH}/{id_func}' @@ -333,10 +338,17 @@ def test_api_v2_lambda_functions_read_wrong_id(self): self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) - @mock.patch('cvat.apps.lambda_manager.views.LambdaGateway._http', return_value = functions["negative"][id_function_non_unique_labels]) - def test_api_v2_lambda_functions_read_non_unique_labels(self, mock_http): - response = self._get_request(f'{LAMBDA_FUNCTIONS_PATH}/{id_function_non_unique_labels}', self.admin) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + def test_api_v2_lambda_functions_read_negative(self): + for id_func in [ + id_function_non_type, id_function_wrong_type, id_function_unknown_type, + id_function_non_unique_labels, + ]: + with mock.patch( + 'cvat.apps.lambda_manager.views.LambdaGateway._http', + return_value=functions["negative"][id_func] + ): + response = self._get_request(f'{LAMBDA_FUNCTIONS_PATH}/{id_func}', self.admin) + self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) @skip("Fail: add mock") @@ -446,8 +458,7 @@ def test_api_v2_lambda_requests_delete_not_finished_request(self): def test_api_v2_lambda_requests_create(self): ids_functions = [id_function_detector, id_function_interactor, id_function_tracker, \ - id_function_reid_with_response_data, id_function_detector, id_function_reid_with_no_response_data, \ - id_function_non_type, id_function_wrong_type, id_function_unknown_type] + id_function_reid_with_response_data, id_function_detector, id_function_reid_with_no_response_data] for id_func in ids_functions: data_main_task = { @@ -492,19 +503,26 @@ def test_api_v2_lambda_requests_create(self): self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) - @mock.patch('cvat.apps.lambda_manager.views.LambdaGateway._http', return_value = functions["negative"]["test-model-has-non-unique-labels"]) - def test_api_v2_lambda_requests_create_non_unique_labels(self, mock_http): - data = { - "function": id_function_non_unique_labels, - "task": self.main_task["id"], - "cleanup": True, - "mapping": { - "car": { "name": "car" }, - }, - } + def test_api_v2_lambda_requests_create_negative(self): + for id_func in [ + id_function_non_type, id_function_wrong_type, id_function_unknown_type, + id_function_non_unique_labels, + ]: + data = { + "function": id_func, + "task": self.main_task["id"], + "cleanup": True, + "mapping": { + "car": { "name": "car" }, + }, + } - response = self._post_request(LAMBDA_REQUESTS_PATH, self.admin, data) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + with mock.patch( + 'cvat.apps.lambda_manager.views.LambdaGateway._http', + return_value=functions["negative"][id_func], + ): + response = self._post_request(LAMBDA_REQUESTS_PATH, self.admin, data) + self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) def test_api_v2_lambda_requests_create_empty_data(self): @@ -808,7 +826,7 @@ def test_api_v2_lambda_functions_create_reid(self): self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) - def test_api_v2_lambda_functions_create_non_type(self): + def test_api_v2_lambda_functions_create_negative(self): data = { "task": self.main_task["id"], "frame": 0, @@ -818,51 +836,16 @@ def test_api_v2_lambda_functions_create_non_type(self): }, } - response = self._post_request(f"{LAMBDA_FUNCTIONS_PATH}/{id_function_non_type}", self.admin, data) - self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) - - - def test_api_v2_lambda_functions_create_wrong_type(self): - data = { - "task": self.main_task["id"], - "frame": 0, - "cleanup": True, - "mapping": { - "car": { "name": "car" }, - }, - } - - response = self._post_request(f"{LAMBDA_FUNCTIONS_PATH}/{id_function_wrong_type}", self.admin, data) - self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) - - - def test_api_v2_lambda_functions_create_unknown_type(self): - data = { - "task": self.main_task["id"], - "frame": 0, - "cleanup": True, - "mapping": { - "car": { "name": "car" }, - }, - } - - response = self._post_request(f"{LAMBDA_FUNCTIONS_PATH}/{id_function_unknown_type}", self.admin, data) - self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) - - - @mock.patch('cvat.apps.lambda_manager.views.LambdaGateway._http', return_value = functions["negative"]["test-model-has-non-unique-labels"]) - def test_api_v2_lambda_functions_create_non_unique_labels(self, mock_http): - data = { - "task": self.main_task["id"], - "frame": 0, - "cleanup": True, - "mapping": { - "car": { "name": "car" }, - }, - } - - response = self._post_request(f"{LAMBDA_FUNCTIONS_PATH}/{id_function_non_unique_labels}", self.admin, data) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + for id_func in [ + id_function_non_type, id_function_wrong_type, id_function_unknown_type, + id_function_non_unique_labels, + ]: + with mock.patch( + 'cvat.apps.lambda_manager.views.LambdaGateway._http', + return_value=functions["negative"][id_func] + ): + response = self._post_request(f"{LAMBDA_FUNCTIONS_PATH}/{id_func}", self.admin, data) + self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) def test_api_v2_lambda_functions_create_quality(self): diff --git a/cvat/apps/lambda_manager/views.py b/cvat/apps/lambda_manager/views.py index e22441e0537..9336f33ee5e 100644 --- a/cvat/apps/lambda_manager/views.py +++ b/cvat/apps/lambda_manager/views.py @@ -43,17 +43,18 @@ from cvat.apps.lambda_manager.serializers import ( FunctionCallRequestSerializer, FunctionCallSerializer ) +from cvat.apps.engine.log import ServerLogManager from cvat.apps.engine.utils import define_dependent_job, get_rq_job_meta, get_rq_lock_by_user from cvat.utils.http import make_requests_session from cvat.apps.iam.filters import ORGANIZATION_OPEN_API_PARAMETERS +slogger = ServerLogManager(__name__) class LambdaType(Enum): DETECTOR = "detector" INTERACTOR = "interactor" REID = "reid" TRACKER = "tracker" - UNKNOWN = "unknown" def __str__(self): return self.value @@ -93,8 +94,11 @@ def _http(self, method="get", scheme=None, host=None, port=None, def list(self): data = self._http(url=self.NUCLIO_ROOT_URL) - response = [LambdaFunction(self, item) for item in data.values()] - return response + for item in data.values(): + try: + yield LambdaFunction(self, item) + except InvalidFunctionMetadataError: + slogger.glob.error("Failed to parse lambda function metadata", exc_info=True) def get(self, func_id): data = self._http(url=self.NUCLIO_ROOT_URL + '/' + func_id) @@ -131,6 +135,9 @@ def _invoke_directly(self, func, payload): return response +class InvalidFunctionMetadataError(Exception): + pass + class LambdaFunction: FRAME_PARAMETERS = ( ('frame', 'frame'), @@ -146,8 +153,9 @@ def __init__(self, gateway, data): kind = meta_anno.get('type') try: self.kind = LambdaType(kind) - except ValueError: - self.kind = LambdaType.UNKNOWN + except ValueError as e: + raise InvalidFunctionMetadataError( + f"{self.id} lambda function has unknown type: {kind!r}") from e # dictionary of labels for the function (e.g. car, person) spec = json.loads(meta_anno.get('spec') or '[]') @@ -160,9 +168,8 @@ def parse_attributes(attrs_spec): } for attr in attrs_spec] if len(parsed_attributes) != len({attr['name'] for attr in attrs_spec}): - raise ValidationError( - f"{self.id} lambda function has non-unique attributes", - code=status.HTTP_404_NOT_FOUND) + raise InvalidFunctionMetadataError( + f"{self.id} lambda function has non-unique attributes") return parsed_attributes @@ -181,9 +188,8 @@ def parse_attributes(attrs_spec): parsed_labels.append(parsed_label) if len(parsed_labels) != len({label['name'] for label in spec}): - raise ValidationError( - f"{self.id} lambda function has non-unique labels", - code=status.HTTP_404_NOT_FOUND) + raise InvalidFunctionMetadataError( + f"{self.id} lambda function has non-unique labels") return parsed_labels @@ -192,9 +198,8 @@ def parse_attributes(attrs_spec): self.func_attributes = {item['name']: item.get('attributes', []) for item in spec} for label, attributes in self.func_attributes.items(): if len([attr['name'] for attr in attributes]) != len(set([attr['name'] for attr in attributes])): - raise ValidationError( - "`{}` lambda function has non-unique attributes for label {}".format(self.id, label), - code=status.HTTP_404_NOT_FOUND) + raise InvalidFunctionMetadataError( + "`{}` lambda function has non-unique attributes for label {}".format(self.id, label)) # description of the function self.description = data['spec']['description'] # http port to access the serverless function