Running migrations automatically if none have run in the past (#1388)

* Running migrations automatically if none have run in the past

* not running automigrate when ignore migrations is truthy, added test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* pre-commit updates

* fixed run migration condition

* removed initilize_db references outside of tests

* Added warning if migrations are ignored

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [UNRELEASED]
 
+### Changed
+
+- Running migrations automatically if none have run in the past (fresh installs, after purging)
+
 ## [0.206.0-rc.0] - 2022-10-26
 
 ### Authors

covalent_dispatcher/_cli/service.py

@@ -319,11 +319,22 @@ def start(
         set_config({"sdk.log_level": "debug"})
 
     db = DataStore.factory()
+
+    # No migrations have run as of yet - run them automatically
+    if not ignore_migrations and db.current_revision() is None:
+        db.run_migrations(logging_enabled=False)
+
     if db.is_migration_pending and not ignore_migrations:
         click.secho(MIGRATION_WARNING_MSG, fg="yellow")
         click.echo(MIGRATION_COMMAND_MSG)
         return ctx.exit(1)
 
+    if ignore_migrations and db.is_migration_pending:
+        click.secho(
+            'Warning: Ignoring migrations is not recommended and may have unanticipated side effects. Use "covalent db migrate" to run migrations.',
+            fg="yellow",
+        )
+
     set_config("user_interface.port", port)
     set_config("dispatcher.port", port)
 

covalent_dispatcher/_db/dispatchdb.py

@@ -31,8 +31,6 @@
 from covalent._shared_files.config import get_config
 from covalent._shared_files.util_classes import Status
 
-from .datastore import DataStore
-
 app_log = logger.app_log
 log_stack_info = logger.log_stack_info
 # DB Schema:
@@ -177,8 +175,3 @@ def __enter__(self):
 
     def __exit__(self, exc_type, exc_value, traceback):
         return False
-
-    def _get_data_store(self, initialize_db: bool = False) -> DataStore:
-        """Return the DataStore instance to write records."""
-
-        return DataStore(db_URL=f"sqlite+pysqlite:///{self._dbpath}", initialize_db=initialize_db)

covalent_ui/app.py

@@ -28,7 +28,6 @@
 
 from covalent._shared_files import logger
 from covalent._shared_files.config import get_config
-from covalent_dispatcher._db.datastore import DataStore
 from covalent_dispatcher._service.app_dask import DaskCluster
 from covalent_ui.api.main import app as fastapi_app
 from covalent_ui.api.main import sio
@@ -102,9 +101,6 @@ def get_home(request: Request, rest_of_path: str):
         dask_cluster = DaskCluster(name="LocalDaskCluster", logger=app_log)
         dask_cluster.start()
 
-    # Initialize the database
-    DataStore(initialize_db=True)
-
     # Start covalent main app
     uvicorn.run(
         "app:fastapi_app",

tests/covalent_dispatcher_tests/_cli/service_test.py

@@ -224,10 +224,19 @@ def test_graceful_shutdown_stopped_server(mocker):
 
 
 @pytest.mark.parametrize(
-    "is_migration_pending, ignore_migrations",
-    [(True, True), (True, False), (False, False), (False, True)],
+    "is_migration_pending, ignore_migrations, current_revision",
+    [
+        (True, True, None),
+        (True, False, None),
+        (False, False, None),
+        (False, True, None),
+        (True, True, "112233"),
+        (True, False, "112233"),
+        (False, False, "112233"),
+        (False, True, "112233"),
+    ],
 )
-def test_start(mocker, monkeypatch, is_migration_pending, ignore_migrations):
+def test_start(mocker, monkeypatch, is_migration_pending, ignore_migrations, current_revision):
     """Test the start CLI command."""
 
     runner = CliRunner()
@@ -245,6 +254,7 @@ def test_start(mocker, monkeypatch, is_migration_pending, ignore_migrations):
     monkeypatch.setattr("covalent_dispatcher._cli.service.UI_LOGFILE", "mock")
 
     db_mock.is_migration_pending = is_migration_pending
+    db_mock.current_revision.return_value = current_revision
 
     cli_args = f"--port {port_val} -d"
 
@@ -253,10 +263,13 @@ def test_start(mocker, monkeypatch, is_migration_pending, ignore_migrations):
 
     res = runner.invoke(start, cli_args)
 
+    # if current_revision is None no migrations have been run
+    if current_revision is None and not ignore_migrations:
+        db_mock.current_revision.assert_called_once()
+
     if ignore_migrations or not is_migration_pending:
         graceful_start_mock.assert_called_once()
         assert set_config_mock.call_count == 6
-        # set_config_mock.assert_called_once()
     else:
         assert MIGRATION_COMMAND_MSG in res.output
         assert MIGRATION_WARNING_MSG in res.output

tests/covalent_dispatcher_tests/_service/app_test.py

@@ -147,7 +147,6 @@ def test_get_result_503(mocker, app, client, test_db_file, tmp_path):
 
 def test_get_result_dispatch_id_not_found(mocker, test_db_file, client, tmp_path):
 
-    mocker.patch.object(DispatchDB, "_get_data_store", test_db_file)
     mocker.patch("covalent_dispatcher._service.app._result_from", return_value={})
     mocker.patch("covalent_dispatcher._service.app.workflow_db", test_db_file)
     mocker.patch("covalent_dispatcher._service.app.Lattice", MockLattice)