Merge pull request #178 from nicholasyager/nyager/fix-versions

Fix: Correct behavior of `version` for models missing YAML properties

dbt_meshify/utilities/versioner.py

@@ -2,6 +2,7 @@
 from typing import Any, Dict, Optional, Union
 
 from dbt.contracts.graph.nodes import ModelNode
+from loguru import logger
 
 from dbt_meshify.change import (
     ChangeSet,
@@ -32,7 +33,11 @@ def __init__(
 
     def load_model_yml(self, path: Path, name: str) -> Dict:
         """Load the Model patch YAML for a given ModelNode."""
-        raw_yml = self.file_manager.read_file(path)
+        try:
+            raw_yml = self.file_manager.read_file(path)
+        except FileNotFoundError as e:
+            logger.debug(f"Unable to load file for versioning. {path}. {e}")
+            return {}
 
         if not isinstance(raw_yml, dict):
             raise Exception(
@@ -68,10 +73,7 @@ def add_version(self, model: ModelNode, defined_in: Optional[Path] = None) -> Ch
         path = self.project.resolve_patch_path(model)
         model_path = self.project.path / model.original_file_path
 
-        try:
-            model_yml = self.load_model_yml(path, model.name)
-        except FileNotFoundError:
-            model_yml = {}
+        model_yml = self.load_model_yml(path, model.name)
 
         model_versions: NamedList = self.get_model_versions(model_yml)
 
@@ -134,19 +136,21 @@ def bump_version(
         path = self.project.resolve_patch_path(model)
         model_path = self.project.path / model.original_file_path
 
-        try:
-            model_yml = self.load_model_yml(path, model.name)
+        model_yml = self.load_model_yml(path, model.name)
+        if model_override:
+            model_yml = safe_update(model_yml, model_override)
 
-            # If a model override has been provided safely update the `model_yml` with the override.
-            if model_override:
-                model_yml = safe_update(model_yml, model_override)
-        except FileNotFoundError:
-            model_yml = {}
-
-        latest_version = model_yml.get("latest_version", 0)
         model_versions: NamedList = self.get_model_versions(model_yml)
         greatest_version = self.get_latest_model_version(model_versions)
 
+        if len(model_versions) == 0:
+            raise ModelVersionerException(
+                f"The model {model.name} does not have any versions defined. Please use add-version first."
+            )
+
+        # Within dbt-core, if unset `latest_version` defaults to the greatest version identifier.
+        latest_version = model_yml.get("latest_version", greatest_version)
+
         # Bump versions
         new_greatest_version_number = greatest_version + 1
         new_latest_version_number = latest_version if prerelease else latest_version + 1
@@ -161,7 +165,7 @@ def bump_version(
         next_version_file_name = model_path.parent / Path(
             f"{defined_in}.{model.language}"
             if defined_in
-            else f"{model.name}_v{new_latest_version_number}.{model.language}"
+            else f"{model.name}_v{new_greatest_version_number}.{model.language}"
         )
 
         change_set = ChangeSet()

tests/integration/test_version_command.py

@@ -1,3 +1,4 @@
+import shutil
 from pathlib import Path
 
 import pytest
@@ -67,7 +68,7 @@ def test_add_version_version_in_yml(
             yaml.safe_dump(start_yml_content, f, sort_keys=False)
     base_command = [
         "operation",
-        "bump-version",
+        "add-version",
         "--select",
         "shared_model",
         "--project-path",
@@ -166,6 +167,45 @@ def test_add_version_version_in_yml_fails_when_versions_present(
             ["shared_model_v1.sql"],
             [],
         ),
+        (
+            None,
+            expected_versioned_model_yml_no_yml,
+            ["shared_model.sql"],
+            ["shared_model_v1.sql"],
+            [],
+        ),
+    ],
+    ids=["1", "2"],
+)
+def test_bump_version_fails_when_no_versions_present(
+    start_yml, end_yml, start_files, expected_files, command_options, project
+):
+    yml_file = proj_path / "models" / "_models.yml"
+    yml_file.parent.mkdir(parents=True, exist_ok=True)
+    runner = CliRunner()
+    # only create file if start_yml is not None
+    # in situations where models don't have a patch path, there isn't a yml file to read from
+    if start_yml:
+        yml_file.touch()
+        start_yml_content = yaml.safe_load(start_yml)
+        with open(yml_file, "w+") as f:
+            yaml.safe_dump(start_yml_content, f, sort_keys=False)
+    base_command = [
+        "operation",
+        "bump-version",
+        "--select",
+        "shared_model",
+        "--project-path",
+        str(proj_path),
+    ]
+    base_command.extend(command_options)
+    result = runner.invoke(cli, base_command)
+    assert result.exit_code != 0
+
+
+@pytest.mark.parametrize(
+    "start_yml,end_yml,start_files,expected_files,command_options",
+    [
         (
             model_yml_increment_version,
             expected_versioned_model_yml_increment_version_no_prerelease,
@@ -187,13 +227,6 @@ def test_add_version_version_in_yml_fails_when_versions_present(
             ["shared_model_v1.sql", "daves_model.sql"],
             ["--defined-in", "daves_model"],
         ),
-        (
-            None,
-            expected_versioned_model_yml_no_yml,
-            ["shared_model.sql"],
-            ["shared_model_v1.sql"],
-            [],
-        ),
         (
             expected_versioned_model_yml_increment_version_with_prerelease,
             expected_versioned_model_yml_increment_prerelease_version_with_second_prerelease,
@@ -209,7 +242,7 @@ def test_add_version_version_in_yml_fails_when_versions_present(
             [],
         ),
     ],
-    ids=["1", "2", "3", "4", "5", "6", "7"],
+    ids=["1", "2", "3", "4", "5"],
 )
 def test_bump_version_in_yml(
     start_yml, end_yml, start_files, expected_files, command_options, project
@@ -234,10 +267,66 @@ def test_bump_version_in_yml(
     ]
     base_command.extend(command_options)
     result = runner.invoke(cli, base_command)
+    print(result.stdout)
+    assert result.exit_code == 0
+    # reset the read path to the default in the logic
+    with open(yml_file, "r") as f:
+        actual = yaml.safe_load(f)
+    for file in expected_files:
+        path = proj_path / "models" / file
+        try:
+            assert path.is_file()
+        except Exception:
+            logger.exception(f"File {file} not found")
+
+    assert actual == yaml.safe_load(end_yml)
+
+
+@pytest.mark.parametrize(
+    "start_yml,end_yml,expected_files,command_options",
+    [
+        (
+            model_yml_increment_version,
+            expected_versioned_model_yml_increment_version_with_prerelease,
+            ["shared_model_v1.sql", "shared_model_v2.sql"],
+            ["--prerelease"],
+        )
+    ],
+)
+def test_bump_version_prerelease_in_yml(
+    start_yml, end_yml, expected_files, command_options, project
+):
+    # Create the original versioned model
+    shutil.move(
+        Path(proj_path / "models" / "shared_model.sql"),
+        Path(proj_path / "models" / "shared_model_v1.sql"),
+    )
+    yml_file = proj_path / "models" / "_models.yml"
+    yml_file.parent.mkdir(parents=True, exist_ok=True)
+    runner = CliRunner()
+    # only create file if start_yml is not None
+    # in situations where models don't have a patch path, there isn't a yml file to read from
+    if start_yml:
+        yml_file.touch()
+        start_yml_content = yaml.safe_load(start_yml)
+        with open(yml_file, "w+") as f:
+            yaml.safe_dump(start_yml_content, f, sort_keys=False)
+    base_command = [
+        "operation",
+        "bump-version",
+        "--select",
+        "shared_model",
+        "--project-path",
+        str(proj_path),
+    ]
+    base_command.extend(command_options)
+    result = runner.invoke(cli, base_command)
+    print(result.stdout)
     assert result.exit_code == 0
     # reset the read path to the default in the logic
     with open(yml_file, "r") as f:
         actual = yaml.safe_load(f)
+
     for file in expected_files:
         path = proj_path / "models" / file
         try:
@@ -299,3 +388,18 @@ def test_command_raises_exception_invalid_paths():
 
     assert result.exit_code != 0
     assert "does not contain a dbt project" in result.stdout
+
+
+def test_version_no_model_yaml(project):
+    """Verify that the version command running in default mode adds versions and creates a second version, too"""
+
+    runner = CliRunner()
+    result = runner.invoke(
+        cli,
+        ["--debug", "version", "--select", "shared_model", "--project-path", proj_path],
+    )
+
+    assert result.exit_code == 0
+
+    assert (proj_path / "models" / "shared_model_v1.sql").exists()
+    assert (proj_path / "models" / "shared_model_v2.sql").exists()