diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ae541c8..a00f8a7 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -2,30 +2,38 @@ default_language_version: python: python3.7 repos: - repo: https://github.com/ambv/black - rev: 19.3b0 + rev: 21.6b0 hooks: - id: black language_version: python3.7 args: ['--target-version', 'py35'] -- repo: https://github.com/pre-commit/pre-commit-hooks - rev: v2.2.3 +- repo: https://github.com/pycqa/flake8 + rev: 3.9.2 hooks: - id: flake8 - additional_dependencies: ["flake8-bugbear==19.3.0"] + additional_dependencies: [ "flake8-bugbear==21.4.3" ] +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.0.1 + hooks: - id: name-tests-test args: ['--django'] - repo: https://github.com/PyCQA/bandit - rev: '1.6.1' + rev: '1.7.0' hooks: - id: bandit exclude: tests/ + args: + - '-s' + - 'B101' # allow use of assert - repo: https://github.com/pre-commit/mirrors-mypy - rev: 'v0.761' + rev: 'v0.902' hooks: - id: mypy exclude: docs/ + additional_dependencies: + - "types-requests~=0.1.9" # type stubs for requests. mypy 0.900 no longer ships these. - repo: https://github.com/pre-commit/mirrors-pylint - rev: 'v2.4.3' + rev: 'v2.7.4' hooks: - id: pylint args: [ diff --git a/.travis.yml b/.travis.yml index b4f0cf7..f2df4c3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -111,6 +111,10 @@ jobs: name: "Python: 3.8 on Linux" python: "3.8" env: TOXENV=py38-cov + - stage: test + name: "Python: 3.9 on Linux" + python: "3.9" + env: TOXENV=py39-cov - stage: test name: "Python: pypy3 on Linux" python: "pypy3" diff --git a/CHANGELOG.md b/CHANGELOG.md index 31a9a2c..81ea335 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.8.0] + ### Added * Add [CONTRIBUTING.md] and [SECURITY.md] [#92] @@ -14,6 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Add `construct_from_cf_env` method to construct client instances from Data Attribute Recommendation service binding on SAP Business Technology Platform. [#97] +* Add support for user-specified Job and Deployment IDs when creating the + respective Job and Deployment resources. This change is not yet generally + available in the Data Attribute Recommendation service. [#98] [CONTRIBUTING.md]: /CONTRIBUTING.md [SECURITY.md]: /SECURITY.md @@ -21,7 +26,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [CII badge details]: https://bestpractices.coreinfrastructure.org/en/projects/4514 [#92]: https://github.com/SAP/data-attribute-recommendation-python-sdk/pull/92 -[#97]: https://github.com/SAP/data-attribute-recommendation-python-sdk/pull/92 +[#97]: https://github.com/SAP/data-attribute-recommendation-python-sdk/pull/97 +[#98]: https://github.com/SAP/data-attribute-recommendation-python-sdk/pull/98 + +### Deprecated + +* Python 3.5 has reached [end-of-life in September 2020](https://www.python.org/downloads/release/python-3510/). + Support for Python 3.5 will be removed in one of the upcoming releases. + ### Changed @@ -154,7 +166,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * First public release -[Unreleased]: https://github.com/SAP/data-attribute-recommendation-python-sdk/compare/rel/0.7.1...HEAD +[Unreleased]: https://github.com/SAP/data-attribute-recommendation-python-sdk/compare/rel/0.8.0...HEAD +[0.8.0]: https://github.com/SAP/data-attribute-recommendation-python-sdk/compare/rel/0.7.1...rel/0.8.0 [0.7.1]: https://github.com/SAP/data-attribute-recommendation-python-sdk/compare/rel/0.7.0...rel/0.7.1 [0.7.0]: https://github.com/SAP/data-attribute-recommendation-python-sdk/compare/rel/0.6.8...rel/0.7.0 [0.6.8]: https://github.com/SAP/data-attribute-recommendation-python-sdk/compare/rel/0.6.7...rel/0.6.8 diff --git a/README.md b/README.md index e95073b..992b2f4 100644 --- a/README.md +++ b/README.md @@ -42,12 +42,15 @@ pay attention to the [CHANGELOG.md]. # Requirements To use the SDK, you will need a recent version of Python. We actively support -and test Python 3.5 up to Python 3.8. We aim to support all officially supported +and test Python ~~3.5~~ 3.6 up to Python 3.8. We aim to support all officially supported Python version. This includes any Python version not listed as `end-of-life` in the [Python Developer's Guide](https://devguide.python.org/#branchstatus). You can check the [Travis builds] to see which environments are actively tested. +**NOTE:** Python 3.5 is [end-of-life since September 2021](https://www.python.org/downloads/release/python-3510/). +The SDK will **remove support** for Python 3.5 at some point after the 0.8.0 release. + Additionally, the `pip` and `virtualenv` tools should be installed. See the [installation instructions][pip and virtual environments]. diff --git a/docs/source/index.rst b/docs/source/index.rst index aa23ad1..577fae0 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -9,7 +9,7 @@ Features - Easy to use - High-level flows on top of the basic Data Attribute Recommendation APIs - Fully type annotated for great autocomplete experience -- Supports Python 3.5 up to 3.8 +- Supports Python 3.6 up to 3.8 (3.5 will be removed in an upcoming release) Release Notes ------------- diff --git a/requirements-dev.txt b/requirements-dev.txt index 2a3c9f2..a9ff724 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,2 +1,2 @@ # see docs/requirements.txt -pre-commit==2.2.0 \ No newline at end of file +pre-commit==2.13.0 \ No newline at end of file diff --git a/requirements-test.txt b/requirements-test.txt index 8298764..7d4097f 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -3,4 +3,4 @@ # whitesource. # These dependencies are never shipped, but only used locally # on a developer's workstation or on Jenkins for development. -tox==3.14.5 +tox==3.23.1 diff --git a/sap/aibus/dar/client/exceptions.py b/sap/aibus/dar/client/exceptions.py index 41c45fc..f192ac4 100644 --- a/sap/aibus/dar/client/exceptions.py +++ b/sap/aibus/dar/client/exceptions.py @@ -32,7 +32,7 @@ class HTTPSRequired(DARException): def __init__(self): msg = "URL must use https scheme. Unencrypted connections are not supported." - super(HTTPSRequired, self).__init__(msg) + super().__init__(msg) class DARPollingTimeoutException(DARException): @@ -129,7 +129,7 @@ def __init__(self, model_name: str): "To re-use the name, please delete the model" " first or choose a different name." ) - super(ModelAlreadyExists, self).__init__(msg) + super().__init__(msg) class DARHTTPException(DARException): @@ -146,7 +146,7 @@ class DARHTTPException(DARException): """ def __init__(self, url: str, response: Response): - super(DARHTTPException, self).__init__() + super().__init__() self.url = url self._response = response self.exception_timestamp = datetime.datetime.now(tz=datetime.timezone.utc) diff --git a/sap/aibus/dar/client/model_manager_client.py b/sap/aibus/dar/client/model_manager_client.py index 5751c97..fc219b5 100644 --- a/sap/aibus/dar/client/model_manager_client.py +++ b/sap/aibus/dar/client/model_manager_client.py @@ -6,6 +6,7 @@ import typing +from uuid import UUID from sap.aibus.dar.client.base_client import BaseClientWithSession from sap.aibus.dar.client.exceptions import ( @@ -131,7 +132,11 @@ def delete_job_by_id(self, job_id: str) -> None: self.session.delete_from_endpoint(endpoint) def create_job( - self, model_name: str, dataset_id: str, model_template_id: str + self, + model_name: str, + dataset_id: str, + model_template_id: str, + job_id: typing.Union[str, UUID] = None, ) -> dict: """ Creates a training Job. @@ -146,9 +151,17 @@ def create_job( A convenience method is available at :meth:`create_job_and_wait` which will submit a job and wait for its completion. + The *job_id* parameter is optional and can be used to specify the ID of the + newly created job. It must be a UUID. + + .. note :: + + The functionality to override the Job ID is not generally available. + :param model_name: Name of the model to train :param dataset_id: Id of previously uploaded, valid dataset :param model_template_id: Model template ID for training + :param job_id: Optionally provide job UUID :return: newly created Job as dict """ self.log.info( @@ -157,13 +170,17 @@ def create_job( dataset_id, model_template_id, ) + + payload = { + "modelName": model_name, + "datasetId": dataset_id, + "modelTemplateId": model_template_id, + } + if job_id: + self.log.info("Job ID override specified: %s", job_id) + payload["id"] = str(job_id) response = self.session.post_to_endpoint( - ModelManagerPaths.ENDPOINT_JOB_COLLECTION, - payload={ - "modelName": model_name, - "datasetId": dataset_id, - "modelTemplateId": model_template_id, - }, + ModelManagerPaths.ENDPOINT_JOB_COLLECTION, payload=payload ) response_as_json = response.json() @@ -171,7 +188,11 @@ def create_job( return response_as_json def create_job_and_wait( - self, model_name: str, dataset_id: str, model_template_id: str + self, + model_name: str, + dataset_id: str, + model_template_id: str, + job_id: typing.Union[str, UUID] = None, ): """ Starts a job and waits for the job to finish. @@ -179,9 +200,17 @@ def create_job_and_wait( This method is a thin wrapper around :meth:`create_job` and :meth:`wait_for_job`. + The *job_id* parameter is optional and can be used to specify the ID of the + newly created job. It must be a UUID. + + .. note :: + + The functionality to override the Job ID is not generally available. + :param model_name: Name of the model to train :param dataset_id: Id of previously uploaded, valid dataset :param model_template_id: Model template ID for training + :param job_id: Optionally provide job UUID :raises TrainingJobFailed: When training job has status FAILED :raises TrainingJobTimeOut: When training job takes too long :return: API response as dict @@ -190,6 +219,7 @@ def create_job_and_wait( model_name=model_name, dataset_id=dataset_id, model_template_id=model_template_id, + job_id=job_id, ) return self.wait_for_job(job_resource["id"]) @@ -226,12 +256,12 @@ def polling_function(): result = polling.poll_until_success( polling_function=polling_function, success_function=self.is_job_finished ) - except PollingTimeoutException: + except PollingTimeoutException as timeout_exception: timeout_msg = "Training job '{}' did not finish within {}s".format( job_id, timeout_seconds ) self.log.exception(timeout_msg) - raise TrainingJobTimeOut(timeout_msg) + raise TrainingJobTimeOut(timeout_msg) from timeout_exception msg = "Job '{}' has status: '{}'".format(job_id, result["status"]) if self.is_job_failed(result): @@ -338,7 +368,7 @@ def read_deployment_by_id(self, deployment_id: str) -> dict: ) return response.json() - def create_deployment(self, model_name: str) -> dict: + def create_deployment(self, model_name: str, deployment_id: str = None) -> dict: """ Creates a Deployment for the given model_name. @@ -349,11 +379,22 @@ def create_deployment(self, model_name: str) -> dict: :meth:`read_deployment_by_id` or the higher-level :meth:`wait_for_deployment` to poll for status changes. + The *deployment_id* parameter is optional and can be used to specify the ID of + the newly created Deployment. + + .. note :: + + The functionality to override the Deployment ID is not generally available. + :param model_name: name of the Model to deploy + :param deployment_id: Optionally provide deployment identifier :return: a single Deployment as dict """ self.log.info("Creating Deployment for model_name '%s'", model_name) payload = {"modelName": model_name} + if deployment_id: + self.log.info("Deployment ID override specified: %s", deployment_id) + payload["id"] = deployment_id response = self.session.post_to_endpoint( ModelManagerPaths.ENDPOINT_DEPLOYMENT_COLLECTION, payload=payload ) @@ -462,20 +503,31 @@ def polling_function(): return response - def deploy_and_wait(self, model_name: str) -> dict: + def deploy_and_wait(self, model_name: str, deployment_id: str = None) -> dict: """ Deploys a Model and waits for Deployment to succeed. This method is a thin wrapper around :meth:`create_deployment` and :meth:`wait_for_deployment`. + The *deployment_id* parameter is optional and can be used to specify the ID of + the newly created Deployment. + + .. note :: + + The functionality to override the Deployment ID is not generally available. + :param model_name: Name of the Model to deploy + :param deployment_id: Optionally provide deployment identifier :raises DeploymentTimeOut: If Deployment does not finish within timeout :raises DeploymentFailed: If Deployment fails :return: Model resource from final API call """ - deployment = self.create_deployment(model_name=model_name) + deployment = self.create_deployment( + model_name=model_name, deployment_id=deployment_id + ) deployment_id = deployment["id"] + assert deployment_id is not None # for mypy self.log.debug( "Created deployment '%s' for model '%s'", deployment_id, model_name ) diff --git a/sap/aibus/dar/client/util/http_transport.py b/sap/aibus/dar/client/util/http_transport.py index d8a941c..6087391 100644 --- a/sap/aibus/dar/client/util/http_transport.py +++ b/sap/aibus/dar/client/util/http_transport.py @@ -175,7 +175,7 @@ def __init__( :param status_forcelist: a set of integer HTTP response codes that will lead to retry. """ - super(RetrySession, self).__init__() + super().__init__() session = session or Session() retry = Retry( total=num_retries, @@ -252,7 +252,7 @@ def __init__( :param connect_timeout: timeout for the connection :param read_timeout: maximum time between bytes after connect """ - super(TimeoutSession, self).__init__() + super().__init__() self.session = session or Session() self.connect_timeout = connect_timeout @@ -301,7 +301,7 @@ def __init__( connect_timeout: connect timeout read_timeout: read timeout """ - super(TimeoutRetrySession, self).__init__() + super().__init__() retry_session = self._make_retry_session(num_retries) timeout_session = TimeoutSession( session=retry_session, diff --git a/setup.py b/setup.py index 8c4594a..b379265 100644 --- a/setup.py +++ b/setup.py @@ -41,6 +41,7 @@ def get_long_version(): "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", + "Programming Language :: Python :: 3.9", "Programming Language :: Python :: Implementation :: CPython", "Programming Language :: Python :: Implementation :: PyPy", "Environment :: Console", diff --git a/tests/sap/aibus/dar/client/test_model_manager_client.py b/tests/sap/aibus/dar/client/test_model_manager_client.py index 2d03bbf..c5630bf 100644 --- a/tests/sap/aibus/dar/client/test_model_manager_client.py +++ b/tests/sap/aibus/dar/client/test_model_manager_client.py @@ -148,6 +148,34 @@ def test_create_job(self, model_manager_client): == response ) + def test_create_job_with_external_job_id(self, model_manager_client): + model_template_id = "d7810207-ca31-4d4d-9b5a-841a644fd81f" + dataset_id = "a2058037-2ae4-465e-8110-65381d47f3d4" + model_name = "my_test_model" + job_id = "24b38880-e304-4d0e-87cf-b9feb8b21393" + + model_manager_client.create_job( + model_template_id=model_template_id, + dataset_id=dataset_id, + model_name=model_name, + job_id=job_id, + ) + + expected_url = "/model-manager/api/v3/jobs" + expected_payload = { + "datasetId": dataset_id, + "modelTemplateId": model_template_id, + "modelName": model_name, + "id": job_id, + } + + expected_post_call = [call(expected_url, payload=expected_payload)] + + assert ( + expected_post_call + == model_manager_client.session.post_to_endpoint.call_args_list + ) + def test_wait_for_job_uses_polling(self, model_manager_client: ModelManagerClient): """ Tests the interaction of the `wait_for_job` method with the Polling class. @@ -239,7 +267,7 @@ def test_wait_for_job_raises_if_job_times_out( assert str(exc_info.value) == expected_message - def test_start_job_and_wait(self, model_manager_client: ModelManagerClient): + def test_create_job_and_wait(self, model_manager_client: ModelManagerClient): """ Tests if start_job_and_wait correctly orchestrates start_job() and wait_for_job(). @@ -271,6 +299,57 @@ def test_start_job_and_wait(self, model_manager_client: ModelManagerClient): model_template_id=model_template_id, dataset_id=dataset_id, model_name=model_name, + job_id=None, + ) + + assert model_manager_client.create_job.call_args_list == [ + expected_create_job_call_args + ] + + expected_wait_for_job_call_args = call(job_resource["id"]) + + assert model_manager_client.wait_for_job.call_args_list == [ + expected_wait_for_job_call_args + ] + + def test_create_job_and_wait_with_external_job_id( + self, model_manager_client: ModelManagerClient + ): + """ + Tests if start_job_and_wait correctly orchestrates start_job() + and wait_for_job() if an external Job ID is used. + """ + model_template_id = "d7810207-ca31-4d4d-9b5a-841a644fd81f" + dataset_id = "a2058037-2ae4-465e-8110-65381d47f3d4" + model_name = "my_test_model" + job_id = "6f122888-3ab9-4485-9b10-d1b982239ab9" + + job_resource = self._make_job_resource("SUCCEEDED") + job_resource["id"] = job_id + + model_manager_client.create_job = create_autospec( + model_manager_client.create_job + ) + model_manager_client.create_job.return_value = job_resource + + model_manager_client.wait_for_job = create_autospec( + model_manager_client.wait_for_job + ) + + ret_val = model_manager_client.create_job_and_wait( + model_name=model_name, + dataset_id=dataset_id, + model_template_id=model_template_id, + job_id=job_id, + ) + + assert ret_val == model_manager_client.wait_for_job.return_value + + expected_create_job_call_args = call( + model_template_id=model_template_id, + dataset_id=dataset_id, + model_name=model_name, + job_id=job_id, ) assert model_manager_client.create_job.call_args_list == [ @@ -466,6 +545,25 @@ def test_create_deployment(self, model_manager_client: ModelManagerClient): == response ) + def test_create_deployment_with_external_id( + self, model_manager_client: ModelManagerClient + ): + model_name = "my_test_model" + deployment_id = "abcd-deployment-id" + + model_manager_client.create_deployment( + model_name=model_name, deployment_id=deployment_id + ) + + expected_url = "/model-manager/api/v3/deployments" + expected_payload = {"modelName": model_name, "id": deployment_id} + expected_post_call = [call(expected_url, payload=expected_payload)] + + assert ( + expected_post_call + == model_manager_client.session.post_to_endpoint.call_args_list + ) + def _make_deployment_resource(self, status): return make_deployment_resource(status) @@ -621,7 +719,55 @@ def test_deploy_and_wait(self, model_manager_client: ModelManagerClient): # assert assert return_value == model_manager_client.wait_for_deployment.return_value - expected_call_to_create_deployment = call(model_name=model_name) + expected_call_to_create_deployment = call( + model_name=model_name, deployment_id=None + ) + + assert model_manager_client.create_deployment.call_args_list == [ + expected_call_to_create_deployment + ] + + expected_call_to_wait_for_deployment = call( + deployment_id=deployment_resource["id"] + ) + + assert model_manager_client.wait_for_deployment.call_args_list == [ + expected_call_to_wait_for_deployment + ] + + def test_deploy_and_wait_with_external_deployment_id( + self, model_manager_client: ModelManagerClient + ): + """ + Tests if *deploy_and_wait* orchestrates *create_deployment* and + *wait_for_deployment* correctly if an external deployment ID is given. + """ + + deployment_id = "abcd" + model_name = "my-test-model" + + # prepare + deployment_resource = self._make_deployment_resource("PENDING") + deployment_resource["id"] = deployment_id + model_manager_client.create_deployment = create_autospec( + model_manager_client.create_deployment, return_value=deployment_resource + ) + + model_manager_client.wait_for_deployment = create_autospec( + model_manager_client.wait_for_deployment + ) + + # act + return_value = model_manager_client.deploy_and_wait( + model_name=model_name, deployment_id=deployment_id + ) + + # assert + assert return_value == model_manager_client.wait_for_deployment.return_value + + expected_call_to_create_deployment = call( + model_name=model_name, deployment_id=deployment_id + ) assert model_manager_client.create_deployment.call_args_list == [ expected_call_to_create_deployment diff --git a/tests/sap/aibus/dar/util/test_http_transport.py b/tests/sap/aibus/dar/util/test_http_transport.py index f1ae957..d77f8b6 100644 --- a/tests/sap/aibus/dar/util/test_http_transport.py +++ b/tests/sap/aibus/dar/util/test_http_transport.py @@ -45,7 +45,7 @@ def test_default_session(self): expected_schemes.remove(scheme) self._assert_retry_set_up_correctly(adapter) - len(expected_schemes) == 0 + assert len(expected_schemes) == 0 def test_can_override_session(self): mock_session = create_autospec(Session, instance=True) @@ -54,7 +54,7 @@ def test_can_override_session(self): assert mock_session.mount.call_count == 2 - session.session == mock_session + assert session.session == mock_session for cal in mock_session.mount.call_args_list: adapter = cal[0][1] diff --git a/tox.ini b/tox.ini index 5ee8d03..234cad3 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] # exclude "system_tests" by default: these need service credentials -envlist = py35, py36, py37, py38 +envlist = py35, py36, py37, py38, py39 [testenv] whitelist_externals = mkdir @@ -11,17 +11,17 @@ passenv = DAR_* TRAVIS TRAVIS_* COVERALLS_* deps = zipp<2.0.0 # for python 3.5 support - pytest==5.3.5 - pytest-cov==2.8.1 - cov: coveralls==2.0.0 - system_tests: pytest-html==2.1.1 + pytest==6.1.2 # latest supporting Python 3.5 + pytest-cov==2.12.1 + cov: coveralls==3.1.0 + system_tests: pytest-html==3.1.1 commands = mkdir -p test_results/{envname}/ pytest \ --doctest-modules \ --cov=sap \ - --cov-fail-under=96 \ + --cov-fail-under=98 \ --cov-report= \ --cov-branch \ --junitxml=test_results/{envname}/unit_xunit.xml \