From 28809b548f900b45047c67094690c53d87f33038 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Mon, 3 Aug 2020 16:06:32 +0200 Subject: [PATCH 1/6] ModelCreator.create: check first if model name is taken In ModelCreator.create, we now perform an initial check to see if the model name is already taken. This prevents long feedback cycles where otherwise the code will first upload a dataset and wait for validation only to fail later on because the model name is already used. Closes #60. --- sap/aibus/dar/client/exceptions.py | 14 +++ sap/aibus/dar/client/workflow/model.py | 14 +++ system_tests/workflow/test_end_to_end.py | 13 ++- tests/sap/aibus/dar/client/test_exceptions.py | 13 ++- tests/sap/aibus/dar/workflow/test_model.py | 94 ++++++++++++++++--- 5 files changed, 132 insertions(+), 16 deletions(-) diff --git a/sap/aibus/dar/client/exceptions.py b/sap/aibus/dar/client/exceptions.py index 9a4b772..31b8c0d 100644 --- a/sap/aibus/dar/client/exceptions.py +++ b/sap/aibus/dar/client/exceptions.py @@ -107,6 +107,20 @@ class DeploymentFailed(DARException): pass +class ModelAlreadyExists(DARException): + """ + Model already exists and must be deleted first. + """ + + def __init__(self, model_name): + msg = "Model '%s' already exists." % model_name + msg += ( + "To re-use the name, please delete the model" + " first or choose a different name." + ) + super(ModelAlreadyExists, self).__init__(msg) + + class DARHTTPException(DARException): """ Error occured when talking to the DAR service over HTTP. diff --git a/sap/aibus/dar/client/workflow/model.py b/sap/aibus/dar/client/workflow/model.py index 2fdfe39..b82210f 100644 --- a/sap/aibus/dar/client/workflow/model.py +++ b/sap/aibus/dar/client/workflow/model.py @@ -6,6 +6,7 @@ from sap.aibus.dar.client.base_client import BaseClient from sap.aibus.dar.client.data_manager_client import DataManagerClient +from sap.aibus.dar.client.exceptions import ModelAlreadyExists, DARHTTPException from sap.aibus.dar.client.model_manager_client import ModelManagerClient from sap.aibus.dar.client.util.credentials import CredentialsSource @@ -62,8 +63,21 @@ def create( :raises: DatasetValidationTimeout: if validation takes too long :raises: DatasetValidationFailed: if validation does not finish in state *SUCCEEDED* + :raises: ModelAlreadyExists: if model already exists at start of process :return: """ + + self.log.info("Checking if model exists") + try: + self.model_manager_client.read_model_by_name(model_name=model_name) + except DARHTTPException as exception: + if exception.status_code == 404: + pass + else: + raise + else: + raise ModelAlreadyExists(model_name) + self.log.info("Creating DatasetSchema.") response_dataset_schema = self.data_manager_client.create_dataset_schema( dataset_schema diff --git a/system_tests/workflow/test_end_to_end.py b/system_tests/workflow/test_end_to_end.py index fb4b51c..e250318 100644 --- a/system_tests/workflow/test_end_to_end.py +++ b/system_tests/workflow/test_end_to_end.py @@ -5,7 +5,7 @@ import pytest from sap.aibus.dar.client.data_manager_client import DataManagerClient -from sap.aibus.dar.client.exceptions import DARHTTPException +from sap.aibus.dar.client.exceptions import DARHTTPException, ModelAlreadyExists from sap.aibus.dar.client.inference_client import InferenceClient from sap.aibus.dar.client.model_manager_client import ModelManagerClient from sap.aibus.dar.client.workflow.model import ModelCreator @@ -86,6 +86,17 @@ def test_create( assert resp["name"] == model_name assert "validationResult" in resp + # Now that the model exists, a second, identical call should + # raise a ModelAlreadyExists exception. + + with pytest.raises(ModelAlreadyExists): + model_creator.create( + model_template_id="d7810207-ca31-4d4d-9b5a-841a644fd81f", + dataset_schema=new_schema, + model_name=model_name, + data_stream=data_stream, + ) + # Check if model is indeed there self._assert_model_exists(model_manager_client, model_name) diff --git a/tests/sap/aibus/dar/client/test_exceptions.py b/tests/sap/aibus/dar/client/test_exceptions.py index 4991d0d..4d0e81c 100644 --- a/tests/sap/aibus/dar/client/test_exceptions.py +++ b/tests/sap/aibus/dar/client/test_exceptions.py @@ -1,7 +1,7 @@ import datetime from unittest.mock import PropertyMock -from sap.aibus.dar.client.exceptions import DARHTTPException +from sap.aibus.dar.client.exceptions import DARHTTPException, ModelAlreadyExists from tests.sap.aibus.dar.client.test_dar_session import create_mock_response # TODO: test __str__ @@ -153,3 +153,14 @@ def test_reason_works_unicode_object(self): exception = DARHTTPException.create_from_response(url, mock_response) assert exception.response_reason == "ÄÖÜ" + + +class TestModelAlreadyExists: + def test_message(self): + e = ModelAlreadyExists(model_name="a-name") + expected_message = "Model 'a-name' already exists." + expected_message += ( + "To re-use the name, please delete the model" + " first or choose a different name." + ) + assert str(e) == expected_message diff --git a/tests/sap/aibus/dar/workflow/test_model.py b/tests/sap/aibus/dar/workflow/test_model.py index 81c71fd..9a7756f 100644 --- a/tests/sap/aibus/dar/workflow/test_model.py +++ b/tests/sap/aibus/dar/workflow/test_model.py @@ -7,6 +7,7 @@ from sap.aibus.dar.client.base_client import BaseClient from sap.aibus.dar.client.data_manager_client import DataManagerClient +from sap.aibus.dar.client.exceptions import ModelAlreadyExists, DARHTTPException from sap.aibus.dar.client.util.credentials import ( StaticCredentialsSource, CredentialsSource, @@ -37,6 +38,31 @@ def csv_data_stream(): return data_stream +@pytest.fixture() +def create_model(): + create_model = ModelCreator.construct_from_jwt("https://abcd/", token="54321") + create_model.data_manager_client = create_autospec(DataManagerClient, instance=True) + create_model.model_manager_client = create_autospec( + ModelManagerClient, instance=True + ) + return create_model + + +@pytest.fixture() +def model_resource(): + return { + "jobId": "522de4e6-2609-4972-8f75-61e9262b86de", + "name": "my-model", + "createdAt": "2018-08-31T11:45:54+00:00", + "validationResult": { + "accuracy": 0.9, + "f1Score": 0.9, + "precision": 0.9, + "recall": 0.9, + }, + } + + a_timestamp = datetime.datetime( 2011, 11, 4, 0, 5, 23, 283000, tzinfo=datetime.timezone.utc ) @@ -108,7 +134,7 @@ def test_format_dataset_name_excessive_length_is_truncated(self): # First part is still all a's assert formatted[:-uuid_len] == input_str[0 : 255 - uuid_len] - def test_create_model(self, csv_data_stream): + def test_create_model(self, csv_data_stream, create_model, model_resource): # inputs # model_name: str, @@ -145,23 +171,22 @@ def test_create_model(self, csv_data_stream): "datasetSchemaId": new_dataset_schema_id, } - create_model = ModelCreator.construct_from_jwt("https://abcd/", token="54321") - create_model.data_manager_client = create_autospec( - DataManagerClient, instance=True - ) - create_model.model_manager_client = create_autospec( - ModelManagerClient, instance=True - ) - create_model.format_dataset_name = Mock(return_value=dataset_name) - dm = create_model.data_manager_client + create_model.data_manager_client.create_dataset_schema.return_value = ( + dataset_schema_created + ) + create_model.data_manager_client.create_dataset.return_value = dataset_created - dm.create_dataset_schema.return_value = dataset_schema_created - dm.create_dataset.return_value = dataset_created + dm = create_model.data_manager_client mm = create_model.model_manager_client + mm.read_model_by_name.side_effect = [ + DARHTTPException(url="https://abcd/", response=Mock(status_code=404)), + model_resource, + ] + # act result = create_model.create( data_stream=csv_data_stream, @@ -170,7 +195,7 @@ def test_create_model(self, csv_data_stream): model_name=model_name, ) - assert result == mm.read_model_by_name.return_value + assert result == model_resource # Expected calls expected_create_dataset_schema = call(dataset_schema) @@ -205,5 +230,46 @@ def test_create_model(self, csv_data_stream): expected_call_to_read_model_by_name = call(model_name=model_name) assert mm.read_model_by_name.call_args_list == [ - expected_call_to_read_model_by_name + expected_call_to_read_model_by_name, + expected_call_to_read_model_by_name, + ] + + def test_create_model_checks_for_existing_model( + self, csv_data_stream, create_model, model_resource + ): + """ + If the model already exists, this should be an error.s + """ + model_name = "my-model" + + model_template_id = "d7810207-ca31-4d4d-9b5a-841a644fd81f" + + dataset_schema = { + "features": [ + {"label": "manufacturer", "type": "CATEGORY"}, + {"label": "description", "type": "TEXT"}, + ], + "labels": [ + {"label": "category", "type": "CATEGORY"}, + {"label": "subcategory", "type": "CATEGORY"}, + ], + "name": "test", + } + + create_model.model_manager_client.read_model_by_name.return_value = ( + model_resource + ) + + with pytest.raises(ModelAlreadyExists) as context: + create_model.create( + data_stream=csv_data_stream, + model_template_id=model_template_id, + dataset_schema=dataset_schema, + model_name=model_name, + ) + + assert "Model 'my-model' already exists" in str(context.value) + + assert create_model.model_manager_client.read_model_by_name.call_args_list == [ + call(model_name=model_name) ] From 19cbbe584a252eac071a09b848197100d1cf1b51 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Mon, 3 Aug 2020 16:14:07 +0200 Subject: [PATCH 2/6] Extend documentation --- sap/aibus/dar/client/exceptions.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/sap/aibus/dar/client/exceptions.py b/sap/aibus/dar/client/exceptions.py index 31b8c0d..41c45fc 100644 --- a/sap/aibus/dar/client/exceptions.py +++ b/sap/aibus/dar/client/exceptions.py @@ -110,9 +110,20 @@ class DeploymentFailed(DARException): class ModelAlreadyExists(DARException): """ Model already exists and must be deleted first. + + Note that this is not really used by the :class:`ModelManagerClient`, but + rather by higher-level methods in :class:`ModelCreator` and similar. + + For methods interacting directly with the API, a request which will + conflict will instead raise a :class:`DARHTTPException` with an appropriate code. """ - def __init__(self, model_name): + def __init__(self, model_name: str): + """ + Constructor. + + :param: model_name: Name of the model which alreadx exists + """ msg = "Model '%s' already exists." % model_name msg += ( "To re-use the name, please delete the model" From 6c4b8cbfecb916e33fdd12bacf5e893dace80656 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Mon, 3 Aug 2020 16:33:54 +0200 Subject: [PATCH 3/6] Add traceability mapping for #60 Mark TestEndToEnd as relevant for #60 --- system_tests/workflow/test_end_to_end.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system_tests/workflow/test_end_to_end.py b/system_tests/workflow/test_end_to_end.py index e250318..4d9330d 100644 --- a/system_tests/workflow/test_end_to_end.py +++ b/system_tests/workflow/test_end_to_end.py @@ -13,7 +13,7 @@ logger = logging.getLogger("test") -@pytest.mark.requirements(issues=["42"]) +@pytest.mark.requirements(issues=["42", "60"]) class TestEndToEnd: """ Tests an end-to-end scenario: From ed0d44e80124fb30b36ae07cb79fcf795715cba6 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Mon, 3 Aug 2020 16:55:44 +0200 Subject: [PATCH 4/6] Bring back system tests We postponed our internal cleanup by a few weeks, so let's bring back the system tests in the meantime so that we can release. --- .travis.yml | 52 ++++++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/.travis.yml b/.travis.yml index 301f41b..eea7612 100644 --- a/.travis.yml +++ b/.travis.yml @@ -59,13 +59,13 @@ script: tox env: global: # DAR_CLIENT_ID - - secure: "eT6zcQaKWSPwycwHRB5o+aO9eSQBOU8ai22sqPOJz580OUA30wT9PJLUeL2R8w91oY6Nkxopm6kmX1hWcrgQUbzGzTfCbuRVu6LdX3tvKuWEcXYAWXbcFne9d15PnKEeHEj59S1+77xXJCLUsgv1AERlHpSFWh9Wy6ZjAsDQX5OxpnOuUQUkRdTWs93swVV280Zcph6oLWMQe42zFqvehagNqdXGYDxd+9cZFvxB4jFk3KBwBXKSlQIaV2oRKQNlKb3S00XNMLGVN3FUMJLwl8PNzlz19nhtllgpqjykd6YeReJXhPVy+NKQ7Syhd0BurShnj2aHZHbtBK+h3Hy3fnJQ+ZNYR4Z0Oj7VWMYlCzGYedfbj6qfKVCEI80eNHAa2jZ/nCOZRgU/0nZCuWwvrqdI6dIVz9j3ep4JS38J/E9V5OTmQNp2sZ6poLvqIENUln3K5g/HhHvjZuoF8/A8/gSOKOfGFb2tVcDFd05B3zd19cpqCx98EeNXBw6r97fKaxcWGTaiXerFlEzMV7XJKpPTuhaPKktssAZwEojSwEjDYIUXtSwkyVdAnNkGHu3EKY9Rk9LOpuMEFzAnH54qiM86IyyIIRhLrnfFm8bomAjD9ouztAbcGrCfJ1dBXzCi//0uu4owhiGwUU+kvbmVRmE6vJ2QK3driWz5BCnWSBM=" + - secure: "RsV5fBK6NZdhQnxQZ2C7sbxfdi5tApcy1+gHG57xWm+fSjX7POXrX3B9S2u0GspCJaSiF6Dqw2Pvx4qlFVOWTDKg7zd0l6BPO4KUVtFiWekagHww4N1QL23R8ChcRwEDuqRK6p5BuBPWwm8TY4aDC5R7XEAFpozYZeb6vsEVflY34PSsy8U8YYjEpLY0tekqNPDqMs9ThpJcfmeVxr8tGYRzExrcWVelm7UXR0e45f5bx5yxL9uDqjHm9C5t78jQllwyNqL7lgx94F0CKncg9l5ob60VK9BzZsy58ITPgF3QxHN+Qk2EIr/n34X/G7yPqT9TxDWtuNhRNxWwSejuvk0OU0zdBMK5684zS1sA19hrl/Lh+iYtpEpkToZamGnIy4c+s1p3AiEbRjKhi+NQemu/SnJf1o0U6ITfyCoK/xvVjpFL7GU3F+B+4FJTz8IMlImWFY8JsR063N4AwyhugI8LoFjWjhD4k7xjf4pmIxQ6CVHgslOvIXBnB1f6hOGbWMhts1JItNOZMt2/6wVH6sUQajVeDyTgSPRXsrLPJvZ9eZ5gTpqx28yvzXd6XkwKrep/LhFM/+EaqADh//z0x96oO6uSkR+yQXFSVzq3UqRY8XQgQFND0xsgfz6ymv+KWX/Zru0I/b3NUg7+8g+iowQILD/o9v7gVDkvFAWxpmU=" # DAR_CLIENT_SECRET - - secure: "YGGg493MQrcOHNgVUZfyDUrXHxRem2vwyDSnXD+5kFrMEGOlui2ICE8ND+/sMwA0QpuCh5oydjLd2IRIAJJylzcBB+ZLvVZ4ez/xVfdoF/uHdjPjETs5xw2HVMxigS8rdygF9h1QxeweluDIG4cSLa8M6bdQHNmfDpXZtjMVT9BxpNRkmOgjAiBe/EhjvSVm74OKAPgsgD8Fpq/tk1sHU0UC3hfR570SR099UgErN1dzjFzB3v8WH8LJcpkmvXXB7KDSInQTGBSIqa97tFlpAcr7SmH3sdAnaEmyfjrER0++PIAMp3DU7frF6tR2L+VJKhyEdFqm36O4s+BPDC7vas+zTzxabb7zytDThMY3Xe7M18xf916w1Uuz6ksrXQZySsS5jzwMDuinLlLXIYTOmct+Iaykix3p59cadqjqbnHb+4ycKyw14YM4ftx0/9nHuWdFN210bIlylDsIULAtZTkWhnrPuin9mLkVIUdvROh3W9WBTmWwE0tOhsRjoaVXh23+SldUJVYerF1hIqh2ETHw0jPTKhcc1BbEeV1Eb3CZNKdwLN6weLJKW60DPZl2oxXZutrXrR1H7aa5m71OX3e/N/udS0R2YFk7VawjIeDN9DW3dWcvqwoZ/cEX8SutGaPQrp2XxedfIPB42gzY+ivJ987amqk1+ODE21EnymM=" + - secure: "cvvfl3dzAuHu5xfcdhSpobOZ4sj4ciKUzPdXTGLCk2lvhl4w08iLyl8MEz+gcRhQYfx2GwTY7QrNAKJxFhW170Uoc6PCg6zMdioHJT55mUo81SZ+cJd+pNWB5cbiTctI7cSG3y+jTjp7QpmLRA0GtrG0TMX3fAVAcXF0cR970X68Vrl9bDYe8cZJvYhkWdxfN1M093/FJ9L1wR9817atL2HrWhlCgkxSaFy+6nIF5RfqCqLxTc5BWoHwm5R/rbx3JrCRhAQZxay8nbnEYRXaIM42jYae0YYWYRTnboYGWtcxwwPey4W71sPTpHfvfS7gPG6dzOEg2JpDujRTcKhzKcsRsqFwWUrg0HUsQZOWvovMP/MIZ8uOQoaDoxHaV8HnT+JHdPcE9RetDqps8WODTpDek2gUTYB7gQeFpBCEW0CF59VWHX6nJrVan0iX/5rFUIYzNkBHtV+grlaXavC2jZ4LojUApM1kMg9ciZxgEr85cQ5H8MhfZLv7mS/T3ZsLr703d+0JGTHos5R18IHN0/MhHHYHo8jWZu02hg7RQU/PQ/jsBJCc178pxY9L1I6fp2HLYJyG9G+3vfMC31soeMB/OqC6r+wPXFH/9a36kwKBN13ad/0a6RXHTUmNOnKwpJLiO2aRi+jCKy78otQwbezz6OMpLN4Ft/+QXcElmmY=" # DAR_AUTH_URL - - secure: "GNzN78NM9I7ky6eUPPLGD7tE+srhaZ4SfslahGy82Yxur3u54RsnkzrJW/s6anc69+OnttKRdgQcFLl3GGOUeeRrnQ48ROgqdf7KIuUSpc0Jzf6fkqOHflwhtMDdVEoxrg+AKTZ2MxsGWVVeL7sWTl0qhQSlNiRHGEzspI49TQ7Gf+wXJKoc9ivJRdEW57zWVzWB0JnJidjrLQ7Ll8A1J1HlWgQlocOKWcpWbmDSPZnHDb7M3xGuY+odVLBSqh45Zki43l/sutufFTg8AcQXvFjN7TJoZBsaavvwanUDcAYpWInmJp+MDAgfqp0P+1VVij34q+rBQa61B6HCBPDa3chzfjPfTUlrUhsAQGCqkpW3SwjLZqtvLSp5wgqiBbiUfQGNG3YPXcjC+5pFdJ6vvdx/QxBtN/CLdiabhdxWiWElHi196RgPdC/+4yhHCq+j26lFks3U1ELU5P48+jEVgoZ7WeaqQvhgOBiMOZ3zoLlHUd15lX9qF2saCI16yoxx5wB7o+FOgy/2klJ/zyy7XZY3nzg0V0pWLe+ZmfAsCqmz1O8ne80tUmcVCRGiChAltE1AxMWjvTr1Vm4D9nIXcdJ6iMEf4LRXbKqr02lmuNI3NsBRK2Za8p7ZTiFZ86AfILt0xxDZpqeD7wYZ8UG6d3IAR8P2UJfPcAwOwqWVAkQ=" + - secure: "bK/Pv9kJWf7GloYFVoNPs4wyx7tDe8TqMHy5VpHFCdMbxPJ9Dn8eH6YmhwYGBsTdkFPU20oQ9dRo7p8VIFKRH84YXoKljawu3Yb97Y3W8k74twWojnoSboiTorinYf46ujJtnuT6Og3r8UPC9eUdjI7jcziOBX3UhmSmCGmjT1P0UvHcgz48eYAJToVZj2/GseDEO7T14nppIGocD4+M6eFbSArDty2GSxjQwm9Xvpbm47KftkRtnTOr4hM1QlD+H+Fo6RBLvBZI10Yhrq59tYph9OvvORFALRWP05DXsQRot5MA/xmtADkAJMN9FcC/XjI7k4zuoPHgy0Uw9Oapfgorfr/fKLd+zFQTAkQeLSHEyv44nflfKjQa/w+cALxRS6JZ+tVmh1ULj4Qo/wR1Dwt4sypL1y7IqOJnC7PjrJmf60jQgkKDtVQVJ0okwz+voZAqlOZFisDbcbIl+NU86TdBZJRQik2YZ94F7LVPy4VEqEKERGqaC83KDhqFgAI8ErNKf+DQKdCugbZXSYMW7tXBokAe9GjXnad8sRihQqIC9SeYgrY554aT7bzjYINFu/IXwkZ3deYeKci85zUskplUCSBjGrlP0wjSn3e+YbzWXc1MXGaqH4BqJb/Xge2p9bw+/6jMwgQIcu0q9/u+k94J4HdBB39brHOMaBbeiCw=" # DAR_URL - - secure: "QCiCCVQCUnOLJHFUp4fxSsu3BeTCL8ny2P3lmljPNmUrCnipQdJKTKQ6tHWQjU2kkprfh/R8z2S1T9vXTWHnMiyhE1sNrsrXa9ePUXN8+i5kqis5sb8fAu5vU++xt5ZVJj/ln6lq7SBr+HQ+l24UZ9H9Te3bS5ggg3Dww0qE17cz4qDc75uoJSV+Z8Or9ivIqlOu3UbeHe5eUEBN8HUZHWmdGhBudxI7UpZwMZeHiUpbat8+hRfuieBPDxATUqLkYugCZdwFmeEMvNEtNyDTbvhT675IMJy0rzSKxHlvHEwjYPqsO+Z4TpdNlkCUbgbD9GahmftSr2cU1DsWGPribi7hNNDoSIjX9PoPUzHO5Idv2P4xsVJVEhClR3ef5VSvvw2v+jvIgKauHGuwn4vYCe7D+xGlWYXITAwCV1dwm0z9A5/3ZsHo0LQQTTXtRmqny/6AQOPu/TfLk0viTR7c/HCvQQa31HtgizrsJ/rA0HYHdb3Uif+N/9CM2jRw2zv24/CwzJND2L7ag8a8ewkixBntQtEvM6/ltUXUhmBaoV3n3WEfmNdmlGN5/TKUgrrs3/eZoN8nQ9P2iTLeSG9SVqk0sh5CQwou16FqQiDGg4k5fAahTHJYry2UPEPTSrMpmTtd8cdGBLXNDZEYW5T0gxXh0xLzLWmVwOMA603ZLPA=" + - secure: "dWwqAu9C8s9opvLUWJr5wfgmSrjKonrTX1IocL4aF5n7RgXM5VnQfgVea86DBqYQcvT6PNsZIvmMjdmpxgit7dkw/RqdHfKLHZE059MFGREj0rHbbe+q+PTJxJtIw8fyUsV1fZ46axzhGeM5ug/9YDX3eXf5m48FElayid1AzYY6mYn2D4+U1dmh+66iEKsb/cRmE3UdoC92dt/VdBxfUKnyC0TI2TexLGybamFQBv/DxC52JYDiaSUwa4BDUu6Mqei//wDwJd73gNIS9lQ9a1FnGM9CBcwrdykaj5Yv8huLfjblfKvZYIl1yMxh0jmhXbJKo+/9zn8aSErXgGG67/1kxyusv4ebu6ivEvIUziKZQS7eE3Cy9xN+5OqHKKzpb1Zuaf+liWsyiqNRLYEsynzB6Rn/RuOg9eMdbaYdjpWby3BG6GF/M999v4ZV5CW3bqxYsq1MErV0CgHytqHDTFEVLvvrqWJTrO78L72xTFV6Jsaxu+lGVo07H+vvOF2JC2432/Kz8PLoG/hTCUyRpBevpv+qt+LVshbTFZKnrCwCLZsWyFA/kwXTcykZl5fF9AjNgyN0oLUH9PGXakNZqBqUVDiwglZYQS3VLt+rgYIcuvSOvsuHnUr+KRT659E6kvU05DJ/ZN9UbWn5yQOyS/FuTJcF28no9JwkdgndxSs=" jobs: include: @@ -76,28 +76,28 @@ jobs: script: - pyenv global 3.7.1 - pre-commit run --all-files -# - stage: system tests -# os: windows -# language: shell # 'language: python' is an error on Travis CI Windows -# before_install: -# - choco install python --version 3.8.2 -# - python -m pip install --upgrade pip -# env: PATH=/c/Python38:/c/Python38/Scripts:$PATH -# # use install: from global test stage, since system tests are -# # also handled via tox -# script: -# - tox -e system_tests -# deploy: -# - provider: releases -# skip_cleanup: true -# api_key: -# # API key for a machine user specifically created for this task: -# # https://github.com/dar-sdk-machine-user -# secure: "m0kXSf53vJY95nb7lrM4N4KTiSmNuSoHfS3swxhjuoEdCPuaYHkpkHjPYH6ZNIjBbFEMGw4YENEcUdyzlshPe/WIX5IQ1LjFTZ1U9wP8Iup0VNW9NAHn32Q9oRUFKdD5IeCtjvQwGdVZbAoJM7+IwX19qqhgkPf1VEPPcq3zW+J/aZNg1ayfH+79x60vybapeG7a8QNvxNATrHaO07+xuWIoDaSKZA0ggDn2zxdaDGk/1SJRZsu67YA0DqFAYB8CtspgewJg7MIrLQcHps9yq+vp1sPHiWy9SH87CDyG9+NMXBb2rnvsmRwxXI659wFOQqMWmCgSs/L3IMPBCGziSqgw7MG5hLVpyrmExINuag2yricm0RDPYVxnJKBz3a7J4pl1zudpdfudich8WDinJk7TJHR3tTgIFx1ASmA20RiY22NUvHWpRL88jNJn5LXDyOaT5bT7c6i9VwxpUq234DXLQ9iUMrbs5P576x/Px71dfJ8RLv52D4TmwQYVzgRdFeFI/TOvyvIKA4dYfGylpsueIScZTn5G0p9srzqpRT8gr1sOlIsBHjISppeyEG6C3uTuhaP/zu7o+soQtiMHWm7G2/5BpUBz7JwMODMnK1f4B4stQJYo7Kx+KsGPUrkJVst07klya44EYGS7Ik8Dm6YsNhvc0safYxxNK71cqGY=" -# file: "system_test_results/traceability.html" -# on: -# tags: true -# condition: $TRAVIS_TAG =~ ^rel/.*$ + - stage: system tests + os: windows + language: shell # 'language: python' is an error on Travis CI Windows + before_install: + - choco install python --version 3.8.2 + - python -m pip install --upgrade pip + env: PATH=/c/Python38:/c/Python38/Scripts:$PATH + # use install: from global test stage, since system tests are + # also handled via tox + script: + - tox -e system_tests + deploy: + - provider: releases + skip_cleanup: true + api_key: + # API key for a machine user specifically created for this task: + # https://github.com/dar-sdk-machine-user + secure: "m0kXSf53vJY95nb7lrM4N4KTiSmNuSoHfS3swxhjuoEdCPuaYHkpkHjPYH6ZNIjBbFEMGw4YENEcUdyzlshPe/WIX5IQ1LjFTZ1U9wP8Iup0VNW9NAHn32Q9oRUFKdD5IeCtjvQwGdVZbAoJM7+IwX19qqhgkPf1VEPPcq3zW+J/aZNg1ayfH+79x60vybapeG7a8QNvxNATrHaO07+xuWIoDaSKZA0ggDn2zxdaDGk/1SJRZsu67YA0DqFAYB8CtspgewJg7MIrLQcHps9yq+vp1sPHiWy9SH87CDyG9+NMXBb2rnvsmRwxXI659wFOQqMWmCgSs/L3IMPBCGziSqgw7MG5hLVpyrmExINuag2yricm0RDPYVxnJKBz3a7J4pl1zudpdfudich8WDinJk7TJHR3tTgIFx1ASmA20RiY22NUvHWpRL88jNJn5LXDyOaT5bT7c6i9VwxpUq234DXLQ9iUMrbs5P576x/Px71dfJ8RLv52D4TmwQYVzgRdFeFI/TOvyvIKA4dYfGylpsueIScZTn5G0p9srzqpRT8gr1sOlIsBHjISppeyEG6C3uTuhaP/zu7o+soQtiMHWm7G2/5BpUBz7JwMODMnK1f4B4stQJYo7Kx+KsGPUrkJVst07klya44EYGS7Ik8Dm6YsNhvc0safYxxNK71cqGY=" + file: "system_test_results/traceability.html" + on: + tags: true + condition: $TRAVIS_TAG =~ ^rel/.*$ - stage: test name: "Python: 3.8 on Windows" # https://docs.travis-ci.com/user/languages/python/#running-python-tests-on-multiple-operating-systems From 596e2739c406687c5165788f0f32c7847d229db2 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Mon, 3 Aug 2020 16:59:00 +0200 Subject: [PATCH 5/6] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91d1a22..21d5140 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed * Documentation: add link to SAP community [#58] +* `ModelCreator.create` performs an initial check if the model name is already used + and raises `ModelAlreadyExists` in this case [#60], [#64] ### Fixed @@ -21,7 +23,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#46]: https://github.com/SAP/data-attribute-recommendation-python-sdk/issues/46 [#58]: https://github.com/SAP/data-attribute-recommendation-python-sdk/pull/58 [#59]: https://github.com/SAP/data-attribute-recommendation-python-sdk/pull/59 +[#60]: https://github.com/SAP/data-attribute-recommendation-python-sdk/issues/60 [#63]: https://github.com/SAP/data-attribute-recommendation-python-sdk/pull/63 +[#64]: https://github.com/SAP/data-attribute-recommendation-python-sdk/pull/64 ## [0.6.8] From dc5f59c864713a809613f2268cdb58446e3e8410 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Mon, 3 Aug 2020 17:12:02 +0200 Subject: [PATCH 6/6] Add missing test case Coveralls complained about line coverage being too low. --- tests/sap/aibus/dar/workflow/test_model.py | 55 +++++++++++++--------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/tests/sap/aibus/dar/workflow/test_model.py b/tests/sap/aibus/dar/workflow/test_model.py index 9a7756f..1a3194d 100644 --- a/tests/sap/aibus/dar/workflow/test_model.py +++ b/tests/sap/aibus/dar/workflow/test_model.py @@ -234,37 +234,21 @@ def test_create_model(self, csv_data_stream, create_model, model_resource): expected_call_to_read_model_by_name, ] - def test_create_model_checks_for_existing_model( - self, csv_data_stream, create_model, model_resource - ): + def test_create_model_checks_for_existing_model(self, create_model, model_resource): """ - If the model already exists, this should be an error.s + If the model already exists, this should be an error. """ model_name = "my-model" - model_template_id = "d7810207-ca31-4d4d-9b5a-841a644fd81f" - - dataset_schema = { - "features": [ - {"label": "manufacturer", "type": "CATEGORY"}, - {"label": "description", "type": "TEXT"}, - ], - "labels": [ - {"label": "category", "type": "CATEGORY"}, - {"label": "subcategory", "type": "CATEGORY"}, - ], - "name": "test", - } - create_model.model_manager_client.read_model_by_name.return_value = ( model_resource ) with pytest.raises(ModelAlreadyExists) as context: create_model.create( - data_stream=csv_data_stream, - model_template_id=model_template_id, - dataset_schema=dataset_schema, + data_stream=Mock(), + model_template_id=Mock(), + dataset_schema=Mock(), model_name=model_name, ) @@ -273,3 +257,32 @@ def test_create_model_checks_for_existing_model( assert create_model.model_manager_client.read_model_by_name.call_args_list == [ call(model_name=model_name) ] + + def test_create_model_forwards_exception(self, create_model, model_resource): + """ + If ModelManagerClient.read_model_by_name raises a 404 in the initial check, + this means that the model is not there and execution and proceed. This is + tested in test_create_model above. + + For all other status code, the exception should be re-raised as is. + This is tested here. + """ + model_name = "my-model" + + exc = DARHTTPException(url="https://abcd/", response=Mock(status_code=429)) + + create_model.model_manager_client.read_model_by_name.side_effect = exc + + with pytest.raises(DARHTTPException) as context: + create_model.create( + data_stream=Mock(), + model_template_id=Mock(), + dataset_schema=Mock(), + model_name=model_name, + ) + + assert context.value == exc + + assert create_model.model_manager_client.read_model_by_name.call_args_list == [ + call(model_name=model_name) + ]