refactor: separate static and dynamic placeholders (#157)

* refactor #108: separate static and dynamic placeholders

Signed-off-by: sushant-suse <[email protected]>

* refactor #108: updated test file

Signed-off-by: sushant-suse <[email protected]>

* refactor #108: align config schema and naming with feedback

Signed-off-by: sushant-suse <[email protected]>

* docs #108: restore and update naming convention comments in env.example.toml

Signed-off-by: sushant-suse <[email protected]>

---------

Signed-off-by: sushant-suse <[email protected]>

Author: sushant-suse

changelog.d/108.refactor.rst

@@ -0,0 +1 @@
+Refactored environment configuration to separate static path placeholders from runtime dynamic placeholders. This prevents invalid directory creation during configuration validation.

etc/docbuild/env.example.toml

@@ -32,6 +32,7 @@ canonical_url_domain = "https://docs.example.com"
 # Key naming convention:
 # * Suffix "_dir": contains only directory paths
 # * Suffix "_path": contains directory and filename paths
+# * Suffix "_dyn": contains placeholders that are replaced at runtime
 root_config_dir = "/etc/docbuild"
 jinja_dir = "{root_config_dir}/jinja-doc-suse-com"
 config_dir = "{root_config_dir}/config.d"
@@ -42,31 +43,38 @@ base_server_cache_dir = "{base_cache_dir}/{server.name}"
 base_tmp_dir = "/var/tmp/docbuild"
 repo_dir = "/data/docserv/repos/permanent-full/"
 tmp_repo_dir = "/data/docserv/repos/temporary-branches/"
-# cache_dir = "{base_cache_dir}/{server.name}"
 meta_cache_dir = "{base_cache_dir}/{server.name}/meta"
 
 
 [paths.tmp]
-# Section "paths.tmp": Definies temporary paths
+# Section "paths.tmp": Defines temporary paths
 # Paths can hold placeholders in brackets.
 tmp_base_dir = "{paths.base_tmp_dir}"
 tmp_dir = "{tmp_base_dir}/doc-example-com"
 tmp_metadata_dir = "{tmp_dir}/metadata"
 tmp_deliverable_dir = "{paths.tmp.tmp_dir}/deliverable/"
-tmp_build_dir = "{tmp_dir}/build/{{product}}-{{docset}}-{{lang}}"
+
+# UPDATED: Split build_dir into base (validated) and dyn (template)
+tmp_build_base_dir = "{tmp_dir}/build"
+tmp_build_dir_dyn = "{{product}}-{{docset}}-{{lang}}"
+
 tmp_out_dir = "{tmp_dir}/out/"
 log_dir = "{tmp_dir}/log/"
-tmp_deliverable_name = "{{product}}_{{docset}}_{{lang}}_XXXXXX"
+
+# UPDATED: Renamed to indicate dynamic template
+tmp_deliverable_name_dyn = "{{product}}_{{docset}}_{{lang}}_XXXXXX"
 
 
 [paths.target]
-# Section "paths.target": Definies target paths
-target_dir = "[email protected]:/srv/docs"
+# Section "paths.target": Defines target paths
+# UPDATED: Split target_dir into base and dyn
+target_base_dir = "[email protected]:/srv/docs"
+target_dir_dyn = "{{product}}"
 backup_dir = "/data/docbuild/external-builds/"
 
 
 [build]
-# Section "build": General build parameters, independant from any specific
+# Section "build": General build parameters, independent from any specific
 
 
 [build.daps]

src/docbuild/cli/cmd_metadata/metaprocess.py

@@ -84,7 +84,7 @@ async def process_deliverable(
     deliverable: Deliverable,
     *,
     repo_dir: Path,
-    temp_repo_dir: Path,
+    tmp_repo_dir: Path,
     base_cache_dir: Path,
     meta_cache_dir: Path,
     dapstmpl: str,
@@ -98,8 +98,8 @@ async def process_deliverable(
     :param deliverable: The Deliverable object to process.
     :param repo_dir: The permanent repo path taken from the env
          config ``paths.repo_dir``
-    :param temp_repo_dir: The temporary repo path taken from the env
-         config ``paths.temp_repo_dir``
+    :param tmp_repo_dir: The temporary repo path taken from the env
+         config ``paths.tmp_repo_dir``
     :param base_cache_dir: The base path of the cache directory taken
          from the env config ``paths.base_cache_dir``
     :param meta_cache_dir: The ath of the metadata directory taken
@@ -132,7 +132,7 @@ async def process_deliverable(
 
     try:
         async with PersistentOnErrorTemporaryDirectory(
-            dir=str(temp_repo_dir),
+            dir=str(tmp_repo_dir),
             prefix=f"clone-{prefix}_",
         ) as worktree_dir:
             # 1. Ensure the bare repository exists/updated using ManagedGitRepo,
@@ -264,7 +264,7 @@ async def process_doctype(
     # if not available:
     meta_cache_dir: Path = env.paths.meta_cache_dir
     # Cloned temporary repo:
-    temp_repo_dir: Path = env.paths.temp_repo_dir
+    tmp_repo_dir: Path = env.paths.tmp_repo_dir
 
     deliverables: list[Deliverable] = await asyncio.to_thread(
         get_deliverable_from_doctype,
@@ -284,7 +284,7 @@ async def process_doctype(
         process_deliverable(
             deliverable,
             repo_dir=repo_dir,
-            temp_repo_dir=temp_repo_dir,
+            tmp_repo_dir=tmp_repo_dir,
             base_cache_dir=base_cache_dir,
             meta_cache_dir=meta_cache_dir,
             dapstmpl=dapsmetatmpl,

src/docbuild/cli/defaults.py

@@ -15,7 +15,7 @@
     "paths": {
         "config_dir": "/etc/docbuild",
         "repo_dir": "/data/docserv/repos/permanent-full/",
-        "temp_repo_dir": "/data/docserv/repos/temporary-branches/",
+        "tmp_repo_dir": "/data/docserv/repos/temporary-branches/",
     },
     "paths.tmp": {
         "tmp_base_dir": "/tmp",
@@ -43,8 +43,7 @@
         "root_config_dir": "/etc/docbuild",
         "jinja_dir": "/etc/docbuild/jinja",
         "server_rootfiles_dir": "/etc/docbuild/root-files",
-        "repo_dir": "/data/docserv/repos/permanent-full/",
-        "temp_repo_dir": "/data/docserv/repos/temporary-branches/",
+        "tmp_repo_dir": "/data/docserv/repos/temporary-branches/",
         "base_cache_dir": "/var/cache/docserv",
         "base_server_cache_dir": "/var/cache/docserv/default",
         "meta_cache_dir": "/var/cache/docserv/default/meta",
@@ -54,13 +53,17 @@
             "tmp_dir": "{tmp_base_dir}/default-local",
             "tmp_deliverable_dir": "{tmp_dir}/deliverable",
             "tmp_metadata_dir": "{tmp_dir}/metadata",
-            "tmp_build_dir": "{tmp_dir}/build/default",
+            # build_dir has become build_base_dir + dir_dyn
+            "tmp_build_base_dir": "{tmp_dir}/build",
             "tmp_out_dir": "{tmp_dir}/out",
             "log_dir": "{tmp_dir}/log",
-            "tmp_deliverable_name": "default_deliverable",
+            # deliverable_name has become name_dyn
+            "tmp_deliverable_name_dyn": "{{product}}_{{docset}}_{{lang}}_XXXXXX",
         },
         "target": {
-            "target_dir": "file:///tmp/docbuild/target",
+            # target_dir has become base_dir + dir_dyn
+            "target_base_dir": "file:///tmp/docbuild/target",
+            "target_dir_dyn": "{{product}}",
             "backup_dir": "/tmp/docbuild/backup",
         },
     },

src/docbuild/models/config/env.py

@@ -205,14 +205,23 @@ class EnvTmpPaths(BaseModel):
     )
     "Temporary metadata directory."
 
-    tmp_build_dir: str = Field(
-        title="Temporary Build Directory",
-        description="Temporary directory for intermediate files (contains placeholders).",
-        examples=[
-            "/var/tmp/docbuild/doc-example-com/build/{{product}}-{{docset}}-{{lang}}"
-        ],
+    # SPLIT: static base directory (validated)
+    tmp_build_base_dir: EnsureWritableDirectory = Field(
+        title="Temporary Build Base Directory",
+        description="The base directory where intermediate build files are stored.",
+        examples=["/var/tmp/docbuild/doc-example-com/build/"],
     )
-    "Temporary build output directory."
+    "Base path for build output."
+
+    # SPLIT: dynamic suffix (string only, not validated as path)
+    # Added a default value so it's not required in defaults.py or user configs
+    tmp_build_dir_dyn: str = Field(
+        default="{{product}}-{{docset}}-{{lang}}",
+        title="Temporary Build Directory Suffix",
+        description="The dynamic part of the build path containing runtime placeholders.",
+        examples=["{{product}}-{{docset}}-{{lang}}"],
+    )
+    "Dynamic suffix for build directory."
 
     tmp_out_dir: EnsureWritableDirectory = Field(
         title="Temporary Output Directory",
@@ -228,28 +237,37 @@ class EnvTmpPaths(BaseModel):
     )
     "Directory for log files."
 
-    tmp_deliverable_name: str = Field(
-        title="Temporary Deliverable Name",
+    # RENAMED: To indicate this is a dynamic template
+    tmp_deliverable_name_dyn: str = Field(
+        title="Temporary Deliverable Name (Dynamic)",
         description=(
-            "The name used for the current deliverable being built "
-            "(e.g., branch name or version)."
+            "The dynamic template name used for the current deliverable being built."
         ),
         examples=["{{product}}_{{docset}}_{{lang}}_XXXXXX"],
     )
-    "Temporary deliverable name."
+    "Temporary deliverable name template."
 
 
 class EnvTargetPaths(BaseModel):
     """Defines target paths."""
 
     model_config = ConfigDict(extra="forbid")
 
-    target_dir: str = Field(
-        title="Target Server Deployment Directory",
-        description="The final remote destination for the built documentation",
+    # SPLIT: static base directory or remote destination
+    target_base_dir: str = Field(
+        title="Target Server Base Directory",
+        description="The static remote destination or base path for built documentation.",
         examples=["[email protected]:/srv/docs"],
     )
-    "The destination directory for final built documentation."
+    "The base destination for final built documentation."
+
+    # SPLIT: dynamic suffix
+    target_dir_dyn: str = Field(
+        title="Target Directory Suffix",
+        description="The dynamic suffix of the remote path containing runtime placeholders.",
+        examples=["{{product}}"],
+    )
+    "Dynamic suffix for final remote destination."
 
     backup_dir: Path = Field(
         title="Build Server Backup Directory",

tests/cli/cmd_config/sample-env.toml

@@ -31,7 +31,7 @@ canonical_url_domain = "https://docs.example.com"
 # Paths can hold placeholders in brackets.
 config_dir = "/etc/docbuild"
 repo_dir = "/data/docserv/repos/permanent-full/"
-temp_repo_dir = "/data/docserv/repos/temporary-branches/"
+tmp_repo_dir = "/data/docserv/repos/temporary-branches/"
 base_cache_dir = "/var/cache/docserv"
 cache_dir = "{base_cache_dir}/{server.name}"
 meta_cache_dir = "{base_cache_dir}/meta"

tests/cli/cmd_metadata/test_cmd_metadata.py

@@ -35,7 +35,7 @@ def mock_envconfig(tmp_path: Path) -> Mock:
     mock_paths.repo_dir = tmp_path / "repos"
     mock_paths.base_cache_dir = tmp_path / "cache"
     mock_paths.meta_cache_dir = tmp_path / "cache" / "metadata"
-    mock_paths.temp_repo_dir = tmp_path / "temp_repos"
+    mock_paths.tmp_repo_dir = tmp_path / "tmp_repos"
 
     mock_build = Mock()
     mock_build.daps.meta = "daps-command-template"
@@ -275,7 +275,7 @@ def setup_paths(self, tmp_path: Path, deliverable: Deliverable) -> dict:
         """Set up common paths and directories for tests."""
         paths = {
             "repo_dir": tmp_path / "repos",
-            "temp_repo_dir": tmp_path / "temp_repos",
+            "tmp_repo_dir": tmp_path / "tmp_repos",
             "base_cache_dir": tmp_path / "cache",
             "meta_cache_dir": tmp_path / "cache" / "metadata",
         }
@@ -460,7 +460,7 @@ async def test_missing_paths_in_config_raises_error(
         # Ensure other paths are set, even if repo_dir is missing
         mock_envconfig.paths.base_cache_dir = Path("/fake/cache")
         mock_envconfig.paths.meta_cache_dir = Path("/fake/cache/meta")
-        mock_envconfig.paths.temp_repo_dir = Path("/fake/temp")
+        mock_envconfig.paths.tmp_repo_dir = Path("/fake/tmp")
         context_missing_path.envconfig = mock_envconfig
 
         with pytest.raises(AttributeError):

tests/conftest.py

@@ -65,7 +65,7 @@ def env_content(default_env_config_filename: Path) -> Path:
 [paths]
 config_dir = "/etc/docbuild"
 repo_dir = "/data/docserv/repos/permanent-full/"
-temp_repo_dir = "/data/docserv/repos/temporary-branches/"
+tmp_repo_dir = "/data/docserv/repos/temporary-branches/"
 
 [paths.tmp]
 tmp_base_dir = "/tmp"