Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

lambda_manager: revamp function parsing error handling #8406

Merged
merged 2 commits into from
Sep 6, 2024
Merged

lambda_manager: revamp function parsing error handling #8406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
746c49a
lambda_manager: revamp function parsing error handling
SpecLad Jun 14, 2024
e588def
Update tests
SpecLad Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions changelog.d/20240905_201903_roman_lambda_error_handling.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
### Changed

- Lambda function endpoints now return 500 instead of 404
if a function's metadata is invalid
(<https://github.com/cvat-ai/cvat/pull/8406>)

- An unknown lambda function type is now treated as invalid metadata
and the function is no longer included in the list endpoint output
(<https://github.com/cvat-ai/cvat/pull/8406>)

### Fixed

- One lambda function with invalid metadata will no longer
break function listing
(<https://github.com/cvat-ai/cvat/pull/8406>)
72 changes: 36 additions & 36 deletions cvat/apps/lambda_manager/tests/assets/functions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
123 changes: 53 additions & 70 deletions cvat/apps/lambda_manager/tests/test_lambda.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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}'
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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,
Expand All @@ -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):
Expand Down
Loading
Loading