Add versioning support for MedPerf APIs (#354)

* Add versioning support for MedPerf APIs * Fix flake8 issues * Fix CI tests * Support versioning from the CLI * Ignore lint error for __init__ * Adapt tests to changes * Fix linter error * Moving API version to common settings * Fix flake8 * Move full url logic to utils function * Pass base url to full_url * Move full url logic to comms * Use API version from settings * Remove deprecated import * Remove deprecated import * make parse_url a class method * Use correct decorator order * use version 0.1.0 * fix version in integration tests --------- Co-authored-by: Alejandro Aristizábal <[email protected]> Co-authored-by: hasan7n <[email protected]>
mlcommons · Mar 16, 2023 · 19b692f · 19b692f
1 parent 96377ee
commit 19b692f
Show file tree

Hide file tree

Showing 32 changed files with 260 additions and 156 deletions.
diff --git a/cli/cli_tests.sh b/cli/cli_tests.sh
@@ -18,6 +18,7 @@ CERT_FILE="${AUTH_CERT:-$(realpath server/cert.crt)}"
 MEDPERF_STORAGE=~/.medperf
 MEDPERF_SUBSTORAGE="$MEDPERF_STORAGE/$(echo $SERVER_URL | cut -d '/' -f 3 | sed -e 's/[.:]/_/g')"
 MEDPERF_LOG_STORAGE="$MEDPERF_SUBSTORAGE/logs/medperf.log"
+VERSION_PREFIX="/api/v0"
 
 echo "Server URL: $SERVER_URL"
 echo "Storage location: $MEDPERF_SUBSTORAGE"
@@ -83,15 +84,15 @@ METRIC_PARAMS="$ASSETS_URL/metrics/mlcube/workspace/parameters.yaml"
 METRICS_SING_IMAGE="$ASSETS_URL/metrics/mlcube/workspace/.image/image.tar.gz"
 
 # admin token
-ADMIN_TOKEN=$(curl -sk -X POST https://127.0.0.1:8000/auth-token/ -d '{"username": "admin", "password": "admin"}' -H 'Content-Type: application/json' | jq -r '.token')
+ADMIN_TOKEN=$(curl -sk -X POST $SERVER_URL$VERSION_PREFIX/auth-token/ -d '{"username": "admin", "password": "admin"}' -H 'Content-Type: application/json' | jq -r '.token')
 
 # create users
 MODELOWNER="mockmodelowner"
 DATAOWNER="mockdataowner"
 BENCHMARKOWNER="mockbenchmarkowner"
-curl -sk -X POST https://127.0.0.1:8000/users/ -d '{"first_name": "model", "last_name": "owner", "username": "'"$MODELOWNER"'", "password": "test", "email": "[email protected]"}' -H 'Content-Type: application/json' -H "Authorization: Token $ADMIN_TOKEN"
-curl -sk -X POST https://127.0.0.1:8000/users/ -d '{"first_name": "bmk", "last_name": "owner", "username": "'"$BENCHMARKOWNER"'", "password": "test", "email": "[email protected]"}' -H 'Content-Type: application/json' -H "Authorization: Token $ADMIN_TOKEN"
-curl -sk -X POST https://127.0.0.1:8000/users/ -d '{"first_name": "data", "last_name": "owner", "username": "'"$DATAOWNER"'", "password": "test", "email": "[email protected]"}' -H 'Content-Type: application/json' -H "Authorization: Token $ADMIN_TOKEN"
+curl -sk -X POST $SERVER_URL$VERSION_PREFIX/users/ -d '{"first_name": "model", "last_name": "owner", "username": "'"$MODELOWNER"'", "password": "test", "email": "[email protected]"}' -H 'Content-Type: application/json' -H "Authorization: Token $ADMIN_TOKEN"
+curl -sk -X POST $SERVER_URL$VERSION_PREFIX/users/ -d '{"first_name": "bmk", "last_name": "owner", "username": "'"$BENCHMARKOWNER"'", "password": "test", "email": "[email protected]"}' -H 'Content-Type: application/json' -H "Authorization: Token $ADMIN_TOKEN"
+curl -sk -X POST $SERVER_URL$VERSION_PREFIX/users/ -d '{"first_name": "data", "last_name": "owner", "username": "'"$DATAOWNER"'", "password": "test", "email": "[email protected]"}' -H 'Content-Type: application/json' -H "Authorization: Token $ADMIN_TOKEN"
 
 ##########################################################
 ################### Start Testing ########################
@@ -184,7 +185,7 @@ medperf benchmark submit --name bmk --description bmk --demo-url $DEMO_URL --dat
 checkFailed "Benchmark submission failed"
 BMK_UID=$(medperf benchmark ls | tail -n 1 | tr -s ' ' | cut -d ' ' -f 2)
 
-curl -sk -X PUT https://127.0.0.1:8000/benchmarks/$BMK_UID/ -d '{"approval_status": "APPROVED"}' -H 'Content-Type: application/json' -H "Authorization: Token $ADMIN_TOKEN"
+curl -sk -X PUT $SERVER_URL$VERSION_PREFIX/benchmarks/$BMK_UID/ -d '{"approval_status": "APPROVED"}' -H 'Content-Type: application/json' -H "Authorization: Token $ADMIN_TOKEN"
 checkFailed "Benchmark approval failed"
 ##########################################################
 

diff --git a/cli/medperf/__init__.py b/cli/medperf/__init__.py
@@ -0,0 +1 @@
+from ._version import __version__  # noqa
diff --git a/cli/medperf/__main__.py b/cli/medperf/__main__.py
@@ -3,6 +3,7 @@
 import logging.handlers
 from os.path import expanduser, abspath
 
+from medperf import __version__
 import medperf.config as config
 from medperf.ui.factory import UIFactory
 from medperf.decorators import clean_except, configurable
@@ -173,11 +174,12 @@ def main(ctx: typer.Context):
     log = config.loglevel.upper()
     log_lvl = getattr(logging, log)
     setup_logging(log_lvl)
+    logging.info(f"Running MedPerf v{__version__} on {log_lvl} logging level")
 
     config.ui = UIFactory.create_ui(config.ui)
     config.comms = CommsFactory.create_comms(config.comms, config.server)
 
-    config.ui.print(f"MedPerf {config.version}")
+    config.ui.print(f"MedPerf {__version__}")
 
 
 if __name__ == "__main__":

diff --git a/cli/medperf/_version.py b/cli/medperf/_version.py
@@ -0,0 +1 @@
+__version__ = "0.1.0"
diff --git a/cli/medperf/comms/interface.py b/cli/medperf/comms/interface.py
@@ -15,6 +15,19 @@ def __init__(self, source: str, ui: UI, token: str = None):
             token (str, Optional): authentication token to be used throughout communication. Defaults to None.
         """
 
+    @classmethod
+    @abstractmethod
+    def parse_url(self, url: str) -> str:
+        """Parse the source URL so that it can be used by the comms implementation.
+        It should handle protocols and versioning to be able to communicate with the API.
+
+        Args:
+            url (str): base URL
+
+        Returns:
+            str: parsed URL with protocol and version
+        """
+
     @abstractmethod
     def login(self, ui: UI):
         """Authenticate the comms instance for further interactions

diff --git a/cli/medperf/comms/rest.py b/cli/medperf/comms/rest.py
@@ -40,20 +40,31 @@ def log_response_error(res, warn=False):
 
 class REST(Comms):
     def __init__(self, source: str, token=None):
-        self.server_url = self.__parse_url(source)
+        self.server_url = self.parse_url(source)
         self.token = token
         self.cert = config.certificate
         if self.cert is None:
             # No certificate provided, default to normal verification
             self.cert = True
 
-    def __parse_url(self, url):
+    @classmethod
+    def parse_url(cls, url: str) -> str:
+        """Parse the source URL so that it can be used by the comms implementation.
+        It should handle protocols and versioning to be able to communicate with the API.
+
+        Args:
+            url (str): base URL
+
+        Returns:
+            str: parsed URL with protocol and version
+        """
         url_sections = url.split("://")
+        api_path = f"/api/v{config.major_version}"
         # Remove protocol if passed
         if len(url_sections) > 1:
             url = "".join(url_sections[1:])
 
-        return f"https://{url}"
+        return f"https://{url}{api_path}"
 
     def login(self, user: str, pwd: str):
         """Authenticates the user with the server. Required for most endpoints

diff --git a/cli/medperf/config.py b/cli/medperf/config.py
@@ -1,6 +1,8 @@
+from ._version import __version__
 from os.path import expanduser, abspath
 
-version = "0.0.0"
+major_version, minor_version, patch_version = __version__.split(".")
+
 server = "https://api.medperf.org"
 certificate = None
 

diff --git a/cli/medperf/setup.py b/cli/medperf/setup.py
diff --git a/cli/medperf/tests/comms/test_rest.py b/cli/medperf/tests/comms/test_rest.py
@@ -10,6 +10,7 @@
 from medperf.tests.mocks import MockResponse
 
 url = "https://mock.url"
+full_url = REST.parse_url(url)
 patch_server = "medperf.comms.rest.{}"
 
 
@@ -22,24 +23,24 @@ def server(mocker, ui):
 @pytest.mark.parametrize(
     "method_params",
     [
-        ("get_benchmark", "get", 200, [1], {}, (f"{url}/benchmarks/1",), {}),
+        ("get_benchmark", "get", 200, [1], {}, (f"{full_url}/benchmarks/1",), {}),
         (
             "get_benchmark_models",
             "get_list",
             200,
             [1],
             [],
-            (f"{url}/benchmarks/1/models",),
+            (f"{full_url}/benchmarks/1/models",),
             {},
         ),
-        ("get_cube_metadata", "get", 200, [1], {}, (f"{url}/mlcubes/1/",), {}),
+        ("get_cube_metadata", "get", 200, [1], {}, (f"{full_url}/mlcubes/1/",), {}),
         (
             "upload_dataset",
             "post",
             201,
             [{}],
             {"id": 1},
-            (f"{url}/datasets/",),
+            (f"{full_url}/datasets/",),
             {"json": {}},
         ),
         (
@@ -48,7 +49,7 @@ def server(mocker, ui):
             201,
             [{}],
             {"id": 1},
-            (f"{url}/results/",),
+            (f"{full_url}/results/",),
             {"json": {}},
         ),
         (
@@ -57,7 +58,7 @@ def server(mocker, ui):
             201,
             [1, 1],
             {},
-            (f"{url}/datasets/benchmarks/",),
+            (f"{full_url}/datasets/benchmarks/",),
             {
                 "json": {
                     "benchmark": 1,
@@ -71,18 +72,18 @@ def server(mocker, ui):
             "_REST__set_approval_status",
             "put",
             200,
-            [f"{url}/mlcubes/1/benchmarks/1", Status.APPROVED.value],
+            [f"{full_url}/mlcubes/1/benchmarks/1", Status.APPROVED.value],
             {},
-            (f"{url}/mlcubes/1/benchmarks/1",),
+            (f"{full_url}/mlcubes/1/benchmarks/1",),
             {"json": {"approval_status": Status.APPROVED.value}},
         ),
         (
             "_REST__set_approval_status",
             "put",
             200,
-            [f"{url}/mlcubes/1/benchmarks/1", Status.REJECTED.value],
+            [f"{full_url}/mlcubes/1/benchmarks/1", Status.REJECTED.value],
             {},
-            (f"{url}/mlcubes/1/benchmarks/1",),
+            (f"{full_url}/mlcubes/1/benchmarks/1",),
             {"json": {"approval_status": Status.REJECTED.value}},
         ),
         (
@@ -91,7 +92,7 @@ def server(mocker, ui):
             200,
             ["pwd"],
             {},
-            (f"{url}/me/password/",),
+            (f"{full_url}/me/password/",),
             {"json": {"password": "pwd"}},
         ),
     ],
@@ -149,7 +150,7 @@ def test_login_with_user_and_pwd(mocker, server, ui, uname, pwd):
     res = MockResponse({"token": ""}, 200)
     spy = mocker.patch("requests.post", return_value=res)
     exp_body = {"username": uname, "password": pwd}
-    exp_path = f"{url}/auth-token/"
+    exp_path = f"{full_url}/auth-token/"
     cert_verify = config.certificate or True
 
     # Act
@@ -249,12 +250,12 @@ def test__req_sanitizes_json(mocker, server):
 def test__get_list_uses_default_page_size(mocker, server):
     # Arrange
     exp_page_size = config.default_page_size
-    exp_url = f"{url}?limit={exp_page_size}&offset=0"
+    exp_url = f"{full_url}?limit={exp_page_size}&offset=0"
     ret_body = MockResponse({"count": 1, "next": None, "results": []}, 200)
     spy = mocker.patch.object(server, "_REST__auth_get", return_value=ret_body)
 
     # Act
-    server._REST__get_list(url)
+    server._REST__get_list(full_url)
 
     # Assert
     spy.assert_called_once_with(exp_url)
@@ -336,7 +337,7 @@ def test_get_benchmarks_calls_benchmarks_path(mocker, server, body):
     bmarks = server.get_benchmarks()
 
     # Assert
-    spy.assert_called_once_with(f"{url}/benchmarks/")
+    spy.assert_called_once_with(f"{full_url}/benchmarks/")
     assert bmarks == [body]
 
 
@@ -367,7 +368,7 @@ def test_get_user_benchmarks_calls_auth_get_for_expected_path(mocker, server):
     server.get_user_benchmarks()
 
     # Assert
-    spy.assert_called_once_with(f"{url}/me/benchmarks/")
+    spy.assert_called_once_with(f"{full_url}/me/benchmarks/")
 
 
 def test_get_user_benchmarks_returns_benchmarks(mocker, server):
@@ -394,7 +395,7 @@ def test_get_mlcubes_calls_mlcubes_path(mocker, server, body):
     cubes = server.get_cubes()
 
     # Assert
-    spy.assert_called_once_with(f"{url}/mlcubes/")
+    spy.assert_called_once_with(f"{full_url}/mlcubes/")
     assert cubes == [body]
 
 
@@ -484,7 +485,7 @@ def test_get_user_cubes_calls_auth_get_for_expected_path(mocker, server):
     server.get_user_cubes()
 
     # Assert
-    spy.assert_called_once_with(f"{url}/me/mlcubes/")
+    spy.assert_called_once_with(f"{full_url}/me/mlcubes/")
 
 
 def test_get_cube_file_calls_download_direct_link_method(mocker, server):
@@ -512,7 +513,7 @@ def test_get_datasets_calls_datasets_path(mocker, server, body):
     dsets = server.get_datasets()
 
     # Assert
-    spy.assert_called_once_with(f"{url}/datasets/")
+    spy.assert_called_once_with(f"{full_url}/datasets/")
     assert dsets == [body]
 
 
@@ -527,7 +528,7 @@ def test_get_dataset_calls_specific_dataset_path(mocker, server, uid, body):
     dset = server.get_dataset(uid)
 
     # Assert
-    spy.assert_called_once_with(f"{url}/datasets/{uid}/")
+    spy.assert_called_once_with(f"{full_url}/datasets/{uid}/")
     assert dset == body
 
 
@@ -543,7 +544,7 @@ def test_get_user_datasets_calls_auth_get_for_expected_path(mocker, server):
     server.get_user_datasets()
 
     # Assert
-    spy.assert_called_once_with(f"{url}/me/datasets/")
+    spy.assert_called_once_with(f"{full_url}/me/datasets/")
 
 
 @pytest.mark.parametrize("body", [{"mlcube": 1}, {}, {"test": "test"}])
@@ -616,7 +617,7 @@ def test_set_dataset_association_approval_sets_approval(
     spy = mocker.patch(
         patch_server.format("REST._REST__set_approval_status"), return_value=res
     )
-    exp_url = f"{url}/datasets/{dataset_uid}/benchmarks/{benchmark_uid}/"
+    exp_url = f"{full_url}/datasets/{dataset_uid}/benchmarks/{benchmark_uid}/"
 
     # Act
     server.set_dataset_association_approval(benchmark_uid, dataset_uid, status)
@@ -636,7 +637,7 @@ def test_set_mlcube_association_approval_sets_approval(
     spy = mocker.patch(
         patch_server.format("REST._REST__set_approval_status"), return_value=res
     )
-    exp_url = f"{url}/mlcubes/{mlcube_uid}/benchmarks/{benchmark_uid}/"
+    exp_url = f"{full_url}/mlcubes/{mlcube_uid}/benchmarks/{benchmark_uid}/"
 
     # Act
     server.set_mlcube_association_approval(benchmark_uid, mlcube_uid, status)
@@ -648,7 +649,7 @@ def test_set_mlcube_association_approval_sets_approval(
 def test_get_datasets_associations_gets_associations(mocker, server):
     # Arrange
     spy = mocker.patch(patch_server.format("REST._REST__get_list"), return_value=[])
-    exp_path = f"{url}/me/datasets/associations/"
+    exp_path = f"{full_url}/me/datasets/associations/"
 
     # Act
     server.get_datasets_associations()
@@ -660,7 +661,7 @@ def test_get_datasets_associations_gets_associations(mocker, server):
 def test_get_cubes_associations_gets_associations(mocker, server):
     # Arrange
     spy = mocker.patch(patch_server.format("REST._REST__get_list"), return_value=[])
-    exp_path = f"{url}/me/mlcubes/associations/"
+    exp_path = f"{full_url}/me/mlcubes/associations/"
 
     # Act
     server.get_cubes_associations()
@@ -675,7 +676,7 @@ def test_get_result_calls_specified_path(mocker, server, uid, body):
     # Arrange
     res = MockResponse(body, 200)
     spy = mocker.patch(patch_server.format("REST._REST__auth_get"), return_value=res)
-    exp_path = f"{url}/results/{uid}/"
+    exp_path = f"{full_url}/results/{uid}/"
 
     # Act
     result = server.get_result(uid)
@@ -707,7 +708,7 @@ def test_set_mlcube_association_priority_sets_priority(
     # Arrange
     res = MockResponse({}, 200)
     spy = mocker.patch(patch_server.format("REST._REST__auth_put"), return_value=res)
-    exp_url = f"{url}/mlcubes/{mlcube_uid}/benchmarks/{benchmark_uid}/"
+    exp_url = f"{full_url}/mlcubes/{mlcube_uid}/benchmarks/{benchmark_uid}/"
 
     # Act
     server.set_mlcube_association_priority(benchmark_uid, mlcube_uid, priority)

diff --git a/cli/medperf/utils.py b/cli/medperf/utils.py
@@ -41,7 +41,6 @@ def setup_logging(log_lvl):
     requests_logger = logging.getLogger("requests")
     requests_logger.addHandler(handler)
     requests_logger.setLevel(log_lvl)
-    logging.info(f"Running MedPerf v{config.version} on {log_lvl} logging level")
 
 
 def delete_credentials():