fix: Move version validation to init, log warning instead of raisin…

…g exception (#708) Add test for #707.
rungalileo · Jul 7, 2023 · a30bf68 · a30bf68
1 parent c51eedd
commit a30bf68
Show file tree

Hide file tree

Showing 25 changed files with 70 additions and 91 deletions.
diff --git a/dataquality/__init__.py b/dataquality/__init__.py
@@ -31,7 +31,7 @@
 """
 
 
-__version__ = "v0.9.3"
+__version__ = "0.9.4"
 
 import sys
 from typing import Any, List, Optional

diff --git a/dataquality/core/finish.py b/dataquality/core/finish.py
@@ -14,7 +14,6 @@
 from dataquality.utils.dq_logger import DQ_LOG_FILE_HOME, upload_dq_log_file
 from dataquality.utils.helpers import check_noop, gpu_available
 from dataquality.utils.thread_pool import ThreadPoolManager
-from dataquality.utils.version import _version_check
 
 api_client = ApiClient()
 a = Analytics(ApiClient, config)  # type: ignore
@@ -50,8 +49,6 @@ def finish(
     data_logger = dataquality.get_data_logger()
     data_logger.validate_labels()
 
-    _version_check()
-
     if data_logger.non_inference_logged():
         _reset_run(config.current_project_id, config.current_run_id, config.task_type)
 

diff --git a/dataquality/core/init.py b/dataquality/core/init.py
@@ -29,6 +29,7 @@
 from dataquality.utils.dq_logger import DQ_LOG_FILE_HOME
 from dataquality.utils.helpers import check_noop
 from dataquality.utils.name import validate_name
+from dataquality.utils.version import version_check
 
 api_client = ApiClient()
 
@@ -182,6 +183,7 @@ def init(
     if not api_client.valid_current_user():
         login()
     _check_dq_version()
+    version_check()
     _init = InitManager()
     task_type = BaseGalileoLogger.validate_task(task_type)
     config.task_type = task_type

diff --git a/dataquality/utils/version.py b/dataquality/utils/version.py
@@ -2,12 +2,12 @@
 
 from dataquality import __version__ as dq_client_version
 from dataquality.core._config import config
-from dataquality.exceptions import GalileoException
 from dataquality.schemas.route import Route
+from dataquality.utils.dq_logger import get_dq_logger
 
 
-def _version_check() -> None:
-    """_version_check.
+def version_check() -> None:
+    """version_check.
 
     Asserts that the dataquality python client and the api have
     matching major versions.
@@ -19,11 +19,9 @@ def _version_check() -> None:
     """
     client_semver = _get_client_version()
     server_semver = _get_api_version()
-    try:
-        assert _major_version(client_semver) == _major_version(server_semver)
-    except AssertionError:
-        raise GalileoException(
-            "Major mismatch between client, "
+    if _major_version(client_semver) != _major_version(server_semver):
+        get_dq_logger().warning(
+            "Major version mismatched between client, "
             f"{client_semver}, and server {server_semver}."
         )
 

diff --git a/tests/core/test_finish.py b/tests/core/test_finish.py
@@ -45,7 +45,6 @@ def test_get_run_status(mock_client: MagicMock) -> None:
     assert status.get("status") == "in_progress"
 
 
-@mock.patch.object(dataquality.core.finish, "_version_check")
 @mock.patch.object(dataquality.core.finish, "_reset_run")
 @mock.patch.object(dataquality.core.finish, "upload_dq_log_file")
 @mock.patch.object(dataquality.clients.api.ApiClient, "make_request")
@@ -60,7 +59,6 @@ def test_finish_waits_default(
     mock_make_request: MagicMock,
     mock_upload_log_file: MagicMock,
     mock_reset_run: MagicMock,
-    mock_version_check: MagicMock,
     set_test_config,
 ) -> None:
     set_test_config(task_type=TaskType.text_classification)
@@ -71,7 +69,6 @@ def test_finish_waits_default(
     mock_wait_for_run.assert_called_once()
 
 
-@mock.patch.object(dataquality.core.finish, "_version_check")
 @mock.patch.object(dataquality.core.finish, "_reset_run")
 @mock.patch.object(dataquality.core.finish, "upload_dq_log_file")
 @mock.patch.object(dataquality.clients.api.ApiClient, "make_request")
@@ -86,7 +83,6 @@ def test_finish_no_waits_when_false(
     mock_make_request: MagicMock,
     mock_upload_log_file: MagicMock,
     mock_reset_run: MagicMock,
-    mock_version_check: MagicMock,
     set_test_config,
 ) -> None:
     set_test_config(task_type=TaskType.text_classification)
@@ -97,7 +93,6 @@ def test_finish_no_waits_when_false(
     mock_wait_for_run.assert_not_called()
 
 
-@mock.patch.object(dataquality.core.finish, "_version_check")
 @mock.patch.object(dataquality.core.finish, "_reset_run")
 @mock.patch.object(dataquality.core.finish, "upload_dq_log_file")
 @mock.patch.object(dataquality.clients.api.ApiClient, "make_request")
@@ -107,7 +102,6 @@ def test_finish_ignores_missing_inference_name_inframe(
     mock_make_request: MagicMock,
     mock_upload_log_file: MagicMock,
     mock_reset_run: MagicMock,
-    mock_version_check: MagicMock,
     set_test_config: Callable,
     cleanup_after_use: Generator,
 ) -> None:
@@ -132,7 +126,6 @@ def test_finish_ignores_missing_inference_name_inframe(
         dataquality.finish()
 
 
-@mock.patch.object(dataquality.core.finish, "_version_check")
 @mock.patch.object(dataquality.core.finish, "_reset_run")
 @mock.patch.object(dataquality.core.finish, "upload_dq_log_file")
 @mock.patch.object(dataquality.clients.api.ApiClient, "make_request")
@@ -149,7 +142,6 @@ def test_finish_with_conditions(
     mock_make_request: MagicMock,
     mock_upload_log_file: MagicMock,
     mock_reset_run: MagicMock,
-    mock_version_check: MagicMock,
     test_session_vars: TestSessionVariables,
     set_test_config,
 ) -> None:

diff --git a/tests/core/test_init.py b/tests/core/test_init.py
@@ -37,13 +37,15 @@
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name", return_value={})
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=True)
 @patch("dataquality.core.init.create_run_name", return_value="foo")
 def test_init(
     mock_create_run_name: MagicMock,
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -69,13 +71,15 @@ def test_init(
 
 @patch.object(ApiClient, "get_project_by_name")
 @patch.object(ApiClient, "get_project_run_by_name")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=True)
 @patch("dataquality.core.init.create_run_name", return_value="foo")
 def test_init_reset_logger_config(
     mock_create_run_name: MagicMock,
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_get_project_by_name: MagicMock,
     set_test_config: Callable,
@@ -96,13 +100,15 @@ def test_init_reset_logger_config(
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name", return_value={})
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=True)
 @patch("dataquality.core.init.create_run_name", return_value="foo")
 def test_init_new_private_project(
     mock_create_run_name: MagicMock,
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -130,13 +136,15 @@ def test_init_new_private_project(
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name", return_value={})
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=True)
 @patch("dataquality.core.init.create_run_name", return_value="foo")
 def test_init_existing_project(
     mock_create_run_name: MagicMock,
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -165,13 +173,15 @@ def test_init_existing_project(
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name", return_value={})
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=True)
 @patch("dataquality.core.init.create_run_name", return_value="foo")
 def test_init_new_project(
     mock_create_run_name: MagicMock,
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -201,11 +211,13 @@ def test_init_new_project(
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name", return_value={})
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=True)
 def test_init_existing_project_new_run(
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -238,11 +250,13 @@ def test_init_existing_project_new_run(
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name")
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=True)
 def test_init_existing_project_existing_run(
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -275,11 +289,13 @@ def test_init_existing_project_existing_run(
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name", return_value={})
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=True)
 def test_init_new_project_run(
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -310,11 +326,13 @@ def test_init_new_project_run(
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name")
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=True)
 def test_init_only_run(
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -337,13 +355,15 @@ def test_init_only_run(
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name")
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=True)
 @patch("dataquality.core.init.create_run_name", return_value="foo")
 def test_init_project_name_collision(
     mock_create_run_name: MagicMock,
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -387,13 +407,15 @@ def test_init_project_name_collision(
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name")
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=True)
 @patch("dataquality.core.init.create_run_name", return_value="foo")
 def test_init_project_name_collision_fails(
     mock_create_run_name: MagicMock,
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -430,13 +452,15 @@ def test_init_failed_login(mock_login: MagicMock, set_test_config: Callable) ->
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name", return_value={})
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch("dataquality.core.init.login", side_effect=mocked_login)
 @patch("dataquality.core.init.create_run_name", return_value="foo")
 def test_init_successful_login(
     mock_create_run_name: MagicMock,
     mock_login: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -474,6 +498,7 @@ def test_init_expired_token_login(
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name", return_value={})
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(
     dataquality.core.init.ApiClient, "get_current_user", side_effect=GalileoException
@@ -485,6 +510,7 @@ def test_init_expired_token_login_full(
     mock_login: MagicMock,
     mock_current_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -520,6 +546,7 @@ def test_init_invalid_user_login(
 @patch.object(ApiClient, "create_project")
 @patch.object(ApiClient, "get_project_run_by_name", return_value={})
 @patch.object(ApiClient, "create_run")
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=False)
 @patch("dataquality.core.init.login", side_effect=mocked_login)
@@ -529,6 +556,7 @@ def test_init_invalid_user_login_full(
     mock_login: MagicMock,
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_create_run: MagicMock,
     mock_get_project_run_by_name: MagicMock,
     mock_create_project: MagicMock,
@@ -548,11 +576,13 @@ def test_init_invalid_user_login_full(
     assert config.current_project_id == test_session_vars.DEFAULT_PROJECT_ID
 
 
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(dataquality.core.init.ApiClient, "valid_current_user", return_value=True)
 def test_init_bad_task(
     mock_valid_user: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
 ) -> None:
     with pytest.raises(GalileoException) as e:
         dataquality.init(task_type="fake_task_type")
@@ -593,6 +623,7 @@ def test_reconfigure_resets_user_token(
 def test_reconfigure_resets_user_token_login_mocked(
     mock_login: MagicMock, set_test_config: Callable
 ) -> None:
+    os.environ["GALILEO_CONSOLE_URL"] = "https://console.fakecompany.io"
     set_test_config(token="old_token")
     dataquality.configure()
     assert all([config.token == "mock_token", config.token != "old_token"])
@@ -601,6 +632,7 @@ def test_reconfigure_resets_user_token_login_mocked(
 
 @patch("requests.post", side_effect=mocked_create_project_run)
 @patch("requests.get", side_effect=mocked_get_project_run)
+@patch("dataquality.core.init.version_check")
 @patch("dataquality.core.init._check_dq_version")
 @patch.object(
     dq.clients.api.ApiClient,
@@ -630,6 +662,7 @@ def test_bad_names(
     mock_valid_user: MagicMock,
     mock_dq_healthcheck: MagicMock,
     mock_check_dq_version: MagicMock,
+    mock_version_check: MagicMock,
     mock_requests_get: MagicMock,
     mock_requests_post: MagicMock,
     run_name: str,