Path refactoring 2

airflow_src/plugins/common/paths.py

@@ -3,24 +3,24 @@
 from pathlib import Path
 
 from shared.db.models import RawFile
-from shared.keys import InternalPaths
 from shared.path_layout import get_output_folder_rel_path
+from shared.path_views import AIRFLOW_CONTAINER_VIEW, Locations
 
 
 def get_internal_instrument_data_path(instrument_id: str) -> Path:
     """Get internal path for the given instrument.
 
     e.g. /opt/airflow/mounts/instruments/test2
     """
-    return Path(InternalPaths.MOUNTS_PATH) / InternalPaths.INSTRUMENTS / instrument_id
+    return AIRFLOW_CONTAINER_VIEW.resolve(Locations.INSTRUMENTS, instrument_id)
 
 
 def get_internal_backup_path() -> Path:
     """Get internal backup path.
 
     e.g. /opt/airflow/mounts/backup
     """
-    return Path(InternalPaths.MOUNTS_PATH) / InternalPaths.BACKUP
+    return AIRFLOW_CONTAINER_VIEW.resolve(Locations.BACKUP)
 
 
 def get_internal_backup_path_for_instrument(
@@ -30,21 +30,19 @@ def get_internal_backup_path_for_instrument(
 
     e.g. /opt/airflow/mounts/backup/test2
     """
-    return get_internal_backup_path() / instrument_id
+    return AIRFLOW_CONTAINER_VIEW.resolve(Locations.BACKUP, instrument_id)
 
 
 def get_internal_output_path() -> Path:
     """Get absolute internal output path."""
-    return Path(InternalPaths.MOUNTS_PATH) / InternalPaths.OUTPUT
+    return AIRFLOW_CONTAINER_VIEW.resolve(Locations.OUTPUT)
 
 
 def get_internal_output_path_for_raw_file(
     raw_file: RawFile,
     software_type: str | None = None,
 ) -> Path:
     """Get absolute internal output path for the given raw file name."""
-    return (
-        Path(InternalPaths.MOUNTS_PATH)
-        / InternalPaths.OUTPUT
-        / get_output_folder_rel_path(raw_file, software_type)
+    return AIRFLOW_CONTAINER_VIEW.resolve(
+        Locations.OUTPUT, get_output_folder_rel_path(raw_file, software_type)
     )

airflow_src/plugins/jobs/_experimental/file_based_job_handler.py

@@ -13,7 +13,6 @@
 
 # TODO: add unit tests
 import logging
-from pathlib import Path
 
 from airflow.exceptions import AirflowFailException
 from common.keys import JobStates
@@ -22,7 +21,8 @@
 from jobs.job_handler import JobHandler
 
 from shared.db.interface import get_raw_file_by_id
-from shared.keys import InternalPaths, SoftwareTypes
+from shared.keys import SoftwareTypes
+from shared.path_views import AIRFLOW_CONTAINER_VIEW, Locations
 
 
 class FileBasedJobHandler(JobHandler):
@@ -33,8 +33,8 @@ def __init__(self):
         super().__init__()
 
         # is is a bit of a hack to use the output path here, but it avoids another bind mount
-        self._job_submit_dir = (
-            Path(InternalPaths.MOUNTS_PATH) / InternalPaths.OUTPUT / "job_queue"
+        self._job_submit_dir = AIRFLOW_CONTAINER_VIEW.resolve(
+            Locations.OUTPUT, "job_queue"
         )
 
     def start_job(self, quanting_env: QuantingEnv) -> str:

airflow_src/tests/helpers.py

@@ -2,24 +2,33 @@
 
 from collections.abc import Iterator
 from contextlib import contextmanager
+from pathlib import PurePosixPath
 from unittest.mock import patch
 
+from shared.path_views import CLUSTER_VIEW
 from shared.yamlsettings import YAMLSETTINGS, YamlKeys
 
 
 @contextmanager
 def yaml_locations(**paths: str) -> Iterator[None]:
     """Override the `locations` section of the yaml settings, e.g. `yaml_locations(slurm="/path/to/slurm")`.
 
-    Patches the settings that `get_path()` reads, rather than `get_path` at its import site,
-    so tests stay valid when code moves between modules.
+    Patches the settings that `get_path()` reads and the contents of `CLUSTER_VIEW`, rather than
+    either of them at its import site, so tests stay valid when code moves between modules.
     """
-    with patch.dict(
-        YAMLSETTINGS,
-        {
-            YamlKeys.LOCATIONS: {
-                key: {YamlKeys.ABSOLUTE_PATH: path} for key, path in paths.items()
-            }
-        },
+    with (
+        patch.dict(
+            YAMLSETTINGS,
+            {
+                YamlKeys.LOCATIONS: {
+                    key: {YamlKeys.ABSOLUTE_PATH: path} for key, path in paths.items()
+                }
+            },
+        ),
+        patch.object(
+            CLUSTER_VIEW,
+            "_locations",
+            {key: PurePosixPath(path) for key, path in paths.items()},
+        ),
     ):
         yield

airflow_src/tests/plugins/jobs/test_file_based_job_handler.py

@@ -10,7 +10,7 @@
 from common.quanting_env import QuantingEnv
 from jobs._experimental.file_based_job_handler import FileBasedJobHandler
 
-from shared.keys import InternalPaths
+from shared.path_views import AIRFLOW_CONTAINER_VIEW, Locations
 
 
 @pytest.fixture
@@ -47,16 +47,12 @@ def test_init_should_set_job_submit_directory(self) -> None:
         handler = FileBasedJobHandler()
 
         # then
-        expected_path = (
-            Path(InternalPaths.MOUNTS_PATH) / InternalPaths.OUTPUT / "job_queue"
-        )
+        expected_path = AIRFLOW_CONTAINER_VIEW.resolve(Locations.OUTPUT, "job_queue")
         assert handler._job_submit_dir == expected_path
 
-    @patch(
-        "jobs._experimental.file_based_job_handler.Path.open", new_callable=mock_open
-    )
-    @patch("jobs._experimental.file_based_job_handler.Path.mkdir")
-    @patch("jobs._experimental.file_based_job_handler.Path.exists")
+    @patch("pathlib.Path.open", new_callable=mock_open)
+    @patch("pathlib.Path.mkdir")
+    @patch("pathlib.Path.exists")
     def test_start_job_should_create_job_file_when_directory_creation_succeeds(
         self,
         mock_exists: MagicMock,
@@ -89,7 +85,7 @@ def test_start_job_should_create_job_file_when_directory_creation_succeeds(
         for expected_line in expected_content:
             handle.write.assert_any_call(expected_line)
 
-    @patch("jobs._experimental.file_based_job_handler.Path.exists")
+    @patch("pathlib.Path.exists")
     def test_start_job_should_raise_exception_when_job_file_already_exists(
         self, mock_exists: MagicMock, sample_quanting_env: QuantingEnv
     ) -> None:
@@ -106,7 +102,7 @@ def test_start_job_should_raise_exception_when_job_file_already_exists(
     @patch(
         "jobs._experimental.file_based_job_handler.get_internal_output_path_for_raw_file"
     )
-    @patch("jobs._experimental.file_based_job_handler.Path.exists")
+    @patch("pathlib.Path.exists")
     def test_get_job_status_should_return_pending_when_status_file_does_not_exist(
         self,
         mock_exists: MagicMock,
@@ -140,7 +136,7 @@ def test_get_job_status_should_return_pending_when_status_file_does_not_exist(
     @patch(
         "jobs._experimental.file_based_job_handler.get_internal_output_path_for_raw_file"
     )
-    @patch("jobs._experimental.file_based_job_handler.Path.exists")
+    @patch("pathlib.Path.exists")
     def test_get_job_status_should_return_correct_status_based_on_file_content(  # noqa: PLR0913
         self,
         mock_exists: MagicMock,
@@ -159,7 +155,7 @@ def test_get_job_status_should_return_correct_status_based_on_file_content(  # n
 
         # when
         with patch(
-            "jobs._experimental.file_based_job_handler.Path.open",
+            "pathlib.Path.open",
             mock_open(read_data=read_data),
         ):
             status = handler.get_job_status("test_raw_file_123")

design_docs/PATH_HANDLING_DESIGN.md

@@ -11,27 +11,27 @@ Drivers: (a) refactor for clarity, (b) enable a second "cluster view" on a Windo
 Every path in the system is
 
 ```
-frame.base(root) / layout(entity)
+view.base(location) / layout(entity)
 ```
 
 Three axes are conflated today:
 
 | Axis | Values |
 |---|---|
-| **root** | backup, output, settings, software, slurm, instruments, logs |
+| **location** | backup, output, settings, software, slurm, instruments, logs |
 | **layout** | `<instrument>/<YYYY_MM>/<raw_file_id>`, `<project>/[<YYYY_MM>]/out_<raw_file_id>/<software_type>` |
-| **frame** | container, cluster, docker-host, SMB source (+ the new Windows one) |
+| **view** | container, cluster, docker-host, SMB source (+ the new Windows one) |
 
-The frame is chosen *implicitly*, by which accessor a caller happens to import
+The view is chosen *implicitly*, by which accessor a caller happens to import
 (`get_internal_*` vs `get_path`). That is the root cause of §6.1-6.5.
 
 ## 2. Sizing facts
 
-- The **cluster frame** has 6 non-test call sites: `processor_impl.py:122,178,188,272`,
+- The **cluster view** has 6 non-test call sites: `processor_impl.py:122,178,188,272`,
   `handler_impl.py:328`, `job_handler.py:23`.
-- The **container frame** has ~41 call sites across 12 files, but they are uniform and can keep
+- The **container view** has ~41 call sites across 12 files, but they are uniform and can keep
   their current API as thin wrappers.
-- The **host frame** has 1 site: `docker_job_handler.py:186`.
+- The **host view** has 1 site: `docker_job_handler.py:186`.
 
 So the expensive-looking half (container) is the cheap half.
 
@@ -49,65 +49,65 @@ Obtained 2026-09-01:
 
 ### D1 - Extra yaml key, branch on engine
 
-`locations.<root>.windows_absolute_path`; `get_path(key, engine)` picks.
+`locations.<location>.windows_absolute_path`; `get_path(key, engine)` picks.
 
 - **Pro:** hours of work, no migration, no new concepts.
 - **Con:** entrenches every problem in §6. A third view is a third key. `get_path` grows an
   engine argument that propagates to all callers. Still `Path` (posix flavor), so `\\host\share`
   is mangled on the Linux worker. Layout stays duplicated in 3 places.
 - **Rating:** cheapest, and paid for twice.
 
-### D2 - Frame table + layout module  (core refactor)
+### D2 - View table + layout module  (core refactor)
 
-- `layout` module: pure, frame-free, returns `(Root, PurePosixPath)`. Single home for the
+- `layout` module: pure, view-free, returns `(Locations, PurePosixPath)`. Single home for the
   raw-file and output layouts. Kills §6.3 and collapses the four relative anchors (§3.1-3.4)
-  into "root + rel"; `RemovePathProvider`'s `Path()` sentinel becomes an honest `rel="."`
-  on a named root.
-- `frames` module: `Root` enum; `Frame(name, flavor, roots: dict[Root, PurePath])` with
-  `.resolve(root, rel)` and `.has(root)`. Instances `CONTAINER` (from `InternalPaths`),
+  into "location + rel"; `RemovePathProvider`'s `Path()` sentinel becomes an honest `rel="."`
+  on a named location.
+- `views` module: `Locations` constants; `View(name, flavor, locations: dict[str, PurePath])` with
+  `.resolve(location, rel)` and `.has(location)`. Instances `AIRFLOW_CONTAINER_VIEW` (from `InternalPaths`),
   `CLUSTER` (from yaml `locations`), `HOST` (from `locations.general.mounts_path`).
-  A missing root raises naming both frame and root, so the §2 holes become explicit.
+  A missing location raises naming both view and location, so the §2 holes become explicit.
 
 - **Pro:** the ragged matrix of §2 becomes data with a real error instead of a silent hole.
   A new view = one dict + a flavor. Layout defined once. Only the 6 cluster sites must move;
   container helpers stay as one-line wrappers so the 41 sites do not churn. `mount.sh` can later
   be generated from the same table (attacks §6.6).
-- **Con:** new module and vocabulary; does not *statically* prevent frame mixing; does not touch
-  the DB-persisted frames or the webapp TODOs (§5.12, §5.13).
+- **Con:** new module and vocabulary; does not *statically* prevent view mixing; does not touch
+  the DB-persisted views or the webapp TODOs (§5.12, §5.13).
 - **Rating:** best clarity per unit of effort. Chosen as step 1.
 
 ### D3 - D2 + Runner objects
 
 `runners:` in yaml (cf. the `alphakraken.example.part.yaml` sketch), made real:
-`Runner{name, engine, frame, ssh_connection_ids, job_script, root overrides}`;
+`Runner{name, engine, view, ssh_connection_ids, job_script, location overrides}`;
 `Settings.runner` replaces `Settings.job_engine`; `prepare_job` resolves all exported paths
-through `runner.frame`, while Airflow-side I/O keeps the container frame.
+through `runner.view`, while Airflow-side I/O keeps the container view.
 
 - **Pro:** literally answers driver (c) - the nature of the quanting env *is* the runner.
-  A Windows cluster becomes one yaml block (`flavor: windows`, own roots, own script, own SSH
+  A Windows cluster becomes one yaml block (`flavor: windows`, own locations, own script, own SSH
   ids) plus a handler. Also fixes the global `cluster_ssh_*` prefix discovery
   (`common/utils.py:198`), which cannot address two clusters today.
 - **Con:** DB migration `job_engine -> runner` plus webapp settings form; yaml schema grows;
   needs validation that a settings' runner exists.
 - **Rating:** the piece that actually buys the second cluster. D2 without D3 gets clarity but
   not the feature.
 
-### D4 - Typed framed paths (on top of D2)
+### D4 - Typed view-tagged paths (on top of D2)
 
-`FramedPath(frame, path)` or a `NewType` per frame; `QuantingEnv` fields annotated by frame.
+`ViewPath(view, path)` or a `NewType` per view; `QuantingEnv` fields annotated by view.
 
 - **Pro:** kills §6.1 statically; the `internal_` naming convention becomes redundant;
   `_check_content`'s whitelist-by-field-name disappears.
 - **Con:** the type leaks at every pydantic / mongoengine / `str()` / filesystem boundary.
   Ceremony out of proportion to 6 cluster call sites.
-- **Rating:** skip. Revisit beyond ~4 frames.
+- **Rating:** skip. Revisit beyond ~4 views.
 
 ### D5 - Relative-on-the-wire
 
-Export only root-relative paths plus per-root base env vars; the job script joins.
+Export only location-relative paths plus per-location base env vars; the job script joins.
 DB stores relative only; the webapp resolves for display.
 
-- **Pro:** exactly one frame exists in Python. Fixes the persisted-frame problem (§5.12), the
+- **Pro:** exactly one view exists in Python. Fixes the persisted-view problem (§5.12), the
   6 webapp TODOs (§5.13), and `RawFile.backup_base_path` disappears. A new view then costs
   zero Python.
 - **Con:** joins move into bash/PowerShell; `{RAW_FILE_PATH}` placeholder semantics change
@@ -116,7 +116,7 @@ DB stores relative only; the webapp resolves for display.
 
 ## 5. Recommendation
 
-D2 now, D3 next, D4 never (until frames multiply), D5 as a separate later decision.
+D2 now, D3 next, D4 never (until views multiply), D5 as a separate later decision.
 
 ## 6. Constraints any Windows view must satisfy (independent of the option chosen)
 
@@ -128,11 +128,11 @@ D2 now, D3 next, D4 never (until frames multiply), D5 as a separate later decisi
   defence for the bash runner.
 - 6.2 **Path flavor.** Foreign-view paths must be built with `PureWindowsPath` /
   `PurePosixPath`, never `Path`: on the Linux worker `Path("\\\\srv\\share")` is one filename,
-  not a UNC root. The frame must own its flavor, and yaml roots must stay opaque strings so
+  not a UNC location. The view must own its flavor, and yaml locations must stay opaque strings so
   that both UNC and drive letters (§3.2) survive.
 - 6.3 **The job script is bash-only.** `submit_job.sh` uses `sbatch`, `module load`, `md5sum`.
   A Windows runner needs its own script and submitter. That is a Runner concern, not a path
   concern - an argument for D3.
 - 6.4 **`mount.sh`, `docker-compose.yaml` and `InternalPaths` agree by convention only** (§6.6),
   cf. the production `backup` mount-depth finding in `BOYSCOUT_20260901_081142.md`. Once the
-  container frame is a table, a test comparing it to `docker-compose.yaml` is ~20 lines.
+  container view is a table, a test comparing it to `docker-compose.yaml` is ~20 lines.