[CDF-22026] 🧑‍🔧Maintain deploy order in build (#740)

* tests: add back functionality

* fix: maintain order

* build; bump

* docs: documented

* tests: regen

* fix: loading of transformation file

* docs; Document why yaml files first

* fix: Added missing connectionType to module

* fix: try to maintain order

* tests: updated test

* refactor; reduce randomness

* refactor; reduced randomness more

* ci: tmp fix

* Revert "ci: tmp fix"

This reverts commit f592694.

* Update cognite_toolkit/_cdf_tk/constants.py

.gitignore

@@ -173,7 +173,7 @@ celerybeat.pid
 *.sage.py
 
 # Environments
-.env
+*.env
 .venv
 .local/
 env/

CHANGELOG.cdf-tk.md

@@ -15,6 +15,13 @@ Changes are grouped as follows:
 - `Fixed` for any bug fixes.
 - `Security` in case of vulnerabilities.
 
+## TBD
+
+### Fixed
+
+- When running `cdf-tk build`, if you had two files non-YAML files named the same in different modules, or subdirectories
+  in the same module, the Toolkit would overwrite the first file with the second file. This is now fixed.
+
 ## [0.2.10] - 2024-07-03
 
 ### Fixed

cognite_toolkit/_cdf_tk/commands/build.py

@@ -8,7 +8,7 @@
 import shutil
 import sys
 import traceback
-from collections import ChainMap, defaultdict
+from collections import ChainMap, Counter, defaultdict
 from collections.abc import Hashable, Mapping, Sequence
 from dataclasses import dataclass, field
 from pathlib import Path
@@ -376,11 +376,18 @@ def _is_selected_module(relative_module_dir: Path, selected_modules: list[str |
 
     def _to_files_by_resource_directory(self, filepaths: list[Path], module_dir: Path) -> dict[str, ResourceDirectory]:
         # Sort to support 1., 2. etc prefixes
-        def sort_key(p: Path) -> int:
-            if result := re.findall(r"^(\d+)", p.stem):
-                return int(result[0])
+        def sort_key(p: Path) -> tuple[int, int, str]:
+            first = {
+                ".yaml": 0,
+                ".yml": 0,
+            }.get(p.suffix.lower(), 1)
+            # We ensure that the YAML files are sorted before other files.
+            # This is when we add indexes to files. We want to ensure that, for example, a .sql file
+            # with the same name as a .yaml file gets the same index as the .yaml file.
+            if result := INDEX_PATTERN.search(p.stem):
+                return first, int(result.group()[:-1]), p.name
             else:
-                return len(filepaths)
+                return first, len(filepaths) + 1, p.name
 
         # The builder of a module can control the order that resources are deployed by prefixing a number
         # The custom key 'sort_key' is to get the sort on integer and not the string.
@@ -736,8 +743,8 @@ class _BuildState:
     variables_by_module_path: dict[str, dict[str, str]] = field(default_factory=dict)
     source_by_build_path: dict[Path, Path] = field(default_factory=dict)
     hash_by_source_path: dict[Path, str] = field(default_factory=dict)
-    index_by_resource_type_counter: dict[str, int] = field(default_factory=lambda: defaultdict(int))
-    index_by_relative_path: dict[Path, int] = field(default_factory=dict)
+    index_by_resource_type_counter: Counter[str] = field(default_factory=Counter)
+    index_by_filepath_stem: dict[Path, int] = field(default_factory=dict)
     printed_function_warning: bool = False
     ids_by_resource_type: dict[type[ResourceLoader], dict[Hashable, Path]] = field(
         default_factory=lambda: defaultdict(dict)
@@ -768,12 +775,18 @@ def create_destination_path(
         # Get rid of the local index
         filename = INDEX_PATTERN.sub("", filename)
 
-        relative_parent = module_dir.name / source_path.relative_to(module_dir).parent
-        if relative_parent not in self.index_by_relative_path:
+        relative_stem = module_dir.name / source_path.relative_to(module_dir).parent / source_path.stem
+        if relative_stem in self.index_by_filepath_stem:
+            # Ensure extra files (.sql, .pdf) with the same stem gets the same index as the
+            # main YAML file. The Transformation Loader expects this.
+            index = self.index_by_filepath_stem[relative_stem]
+        else:
+            # Increment to ensure we do not get duplicate filenames when we flatten the file
+            # structure from the module to the build directory.
             self.index_by_resource_type_counter[resource_directory] += 1
-            self.index_by_relative_path[relative_parent] = self.index_by_resource_type_counter[resource_directory]
+            index = self.index_by_resource_type_counter[resource_directory]
+            self.index_by_filepath_stem[relative_stem] = index
 
-        index = self.index_by_relative_path[relative_parent]
         filename = f"{index}.{filename}"
         destination_path = build_dir / resource_directory / filename
         destination_path.parent.mkdir(parents=True, exist_ok=True)

cognite_toolkit/_cdf_tk/constants.py

@@ -38,7 +38,6 @@
 COGNITE_MODULES_PATH = ROOT_PATH / COGNITE_MODULES
 
 SUPPORT_MODULE_UPGRADE_FROM_VERSION = "0.1.0"
-
 # This is used in the build directory to keep track of order and flatten the
 # module directory structure with accounting for duplicated names.
 INDEX_PATTERN = re.compile("^[0-9]+\\.")

cognite_toolkit/_cdf_tk/loaders/_resource_loaders.py

@@ -1801,9 +1801,17 @@ def _are_equal(
     def _get_query_file(filepath: Path, transformation_external_id: str | None) -> Path | None:
         query_file = filepath.parent / f"{filepath.stem}.sql"
         if not query_file.exists() and transformation_external_id:
-            query_file = filepath.parent / f"{transformation_external_id}.sql"
-            if not query_file.exists():
+            found_query_file = next(
+                (
+                    f
+                    for f in filepath.parent.iterdir()
+                    if f.is_file() and f.name.endswith(f"{transformation_external_id}.sql")
+                ),
+                None,
+            )
+            if found_query_file is None:
                 return None
+            query_file = found_query_file
         return query_file
 
     def load_resource(

cognite_toolkit/cognite_modules/experimental/example_pump_data_model/data_models/3.LiftStation.view.yaml

@@ -13,6 +13,7 @@ implements:
     version: v1
 properties:
   pumps:
+    connectionType: multi_edge_connection
     type:
       space: '{{model_space}}'
       externalId: LiftStation.pumps

tests/tests_integration/constants.py

@@ -5,4 +5,4 @@
 # This is needed as we run tests for two different versions of Python in parallel.
 # The platform.system() is not used, but is here in case we start testing on Windows as well.
 # The random number is to avoid conflicts when running tests in parallel (for example, two PRs).
-RUN_UNIQUE_ID = f"{platform.system()}_{sys.version_info.major}_{sys.version_info.minor}_{random.randint(0, 10_000)!s}"
+RUN_UNIQUE_ID = f"{platform.system()}_{sys.version_info.major}_{sys.version_info.minor}_{random.randint(0, 10)!s}"

tests/tests_unit/approval_client/client.py

@@ -54,6 +54,8 @@
 from cognite.client.utils._text import to_camel_case
 from requests import Response
 
+from cognite_toolkit._cdf_tk.constants import INDEX_PATTERN
+
 from .config import API_RESOURCES
 from .data_classes import APIResource, AuthGroupCalls
 
@@ -394,11 +396,16 @@ def upload(*args, **kwargs) -> None:
             name = ""
             for k, v in kwargs.items():
                 if isinstance(v, Path) or (isinstance(v, str) and Path(v).exists()):
+                    # The index pattern is used to ensure unique names. This index
+                    # is removed as we do not care whether the order of the files are uploaded
+                    filepath = Path(v)
+                    filepath = filepath.with_name(INDEX_PATTERN.sub("", filepath.name))
+
                     try:
-                        kwargs[k] = "/".join(Path(v).relative_to(TEST_FOLDER).parts)
+                        kwargs[k] = "/".join(filepath.relative_to(TEST_FOLDER).parts)
                     except ValueError:
-                        kwargs[k] = "/".join(Path(v).parts)
-                    name = Path(v).name
+                        kwargs[k] = "/".join(filepath.parts)
+                    name = filepath.name
 
             created_resources[resource_cls.__name__].append(
                 {

tests/tests_unit/test_cdf_tk/test_loaders/test_base_loaders.py

@@ -79,7 +79,6 @@ def test_loader_class(
 
 
 class TestDeployResources:
-    @pytest.mark.skip("This functionality has been removed")
     def test_deploy_resource_order(self, cognite_client_approval: ApprovalCogniteClient):
         build_env_name = "dev"
         system_config = SystemYAML.load_from_directory(PYTEST_PROJECT, build_env_name)

tests/tests_unit/test_cli/test_build_deploy_snapshots/cdf_oid_example_data.yaml

@@ -60,68 +60,68 @@ FileMetadata:
   kwargs:
     external_id: fileshare_PH-25578-P-4110006-001.pdf
     overwrite: true
-    path: 1.PH-25578-P-4110006-001.pdf
-  name: 1.PH-25578-P-4110006-001.pdf
+    path: PH-25578-P-4110006-001.pdf
+  name: PH-25578-P-4110006-001.pdf
 - args: []
   kwargs:
     external_id: fileshare_PH-25578-P-4110010-001.pdf
     overwrite: true
-    path: 1.PH-25578-P-4110010-001.pdf
-  name: 1.PH-25578-P-4110010-001.pdf
+    path: PH-25578-P-4110010-001.pdf
+  name: PH-25578-P-4110010-001.pdf
 - args: []
   kwargs:
     external_id: fileshare_PH-25578-P-4110119-001.pdf
     overwrite: true
-    path: 1.PH-25578-P-4110119-001.pdf
-  name: 1.PH-25578-P-4110119-001.pdf
+    path: PH-25578-P-4110119-001.pdf
+  name: PH-25578-P-4110119-001.pdf
 - args: []
   kwargs:
     external_id: fileshare_PH-ME-P-0003-001.pdf
     overwrite: true
-    path: 1.PH-ME-P-0003-001.pdf
-  name: 1.PH-ME-P-0003-001.pdf
+    path: PH-ME-P-0003-001.pdf
+  name: PH-ME-P-0003-001.pdf
 - args: []
   kwargs:
     external_id: fileshare_PH-ME-P-0004-001.pdf
     overwrite: true
-    path: 1.PH-ME-P-0004-001.pdf
-  name: 1.PH-ME-P-0004-001.pdf
+    path: PH-ME-P-0004-001.pdf
+  name: PH-ME-P-0004-001.pdf
 - args: []
   kwargs:
     external_id: fileshare_PH-ME-P-0151-001.pdf
     overwrite: true
-    path: 1.PH-ME-P-0151-001.pdf
-  name: 1.PH-ME-P-0151-001.pdf
+    path: PH-ME-P-0151-001.pdf
+  name: PH-ME-P-0151-001.pdf
 - args: []
   kwargs:
     external_id: fileshare_PH-ME-P-0152-001.pdf
     overwrite: true
-    path: 1.PH-ME-P-0152-001.pdf
-  name: 1.PH-ME-P-0152-001.pdf
+    path: PH-ME-P-0152-001.pdf
+  name: PH-ME-P-0152-001.pdf
 - args: []
   kwargs:
     external_id: fileshare_PH-ME-P-0153-001.pdf
     overwrite: true
-    path: 1.PH-ME-P-0153-001.pdf
-  name: 1.PH-ME-P-0153-001.pdf
+    path: PH-ME-P-0153-001.pdf
+  name: PH-ME-P-0153-001.pdf
 - args: []
   kwargs:
     external_id: fileshare_PH-ME-P-0156-001.pdf
     overwrite: true
-    path: 1.PH-ME-P-0156-001.pdf
-  name: 1.PH-ME-P-0156-001.pdf
+    path: PH-ME-P-0156-001.pdf
+  name: PH-ME-P-0156-001.pdf
 - args: []
   kwargs:
     external_id: fileshare_PH-ME-P-0156-002.pdf
     overwrite: true
-    path: 1.PH-ME-P-0156-002.pdf
-  name: 1.PH-ME-P-0156-002.pdf
+    path: PH-ME-P-0156-002.pdf
+  name: PH-ME-P-0156-002.pdf
 - args: []
   kwargs:
     external_id: fileshare_PH-ME-P-0160-001.pdf
     overwrite: true
-    path: 1.PH-ME-P-0160-001.pdf
-  name: 1.PH-ME-P-0160-001.pdf
+    path: PH-ME-P-0160-001.pdf
+  name: PH-ME-P-0160-001.pdf
 - dataSetId: 42
   externalId: fileshare_PH-25578-P-4110006-001.pdf
   name: PH-25578-P-4110006-001.pdf