Installables: nested docker image path

[podkidyshev]

Summary

The generated path or complicated docker image URLs was nested (relatively to install dir) - intermediate dirs were not created thus installation failed

The PR makes sure docker image URLs always resolve into file names, not paths

Test Plan

• Automated CI
• Manual run with suggested docker image url

Additional Notes

• Fixes P1 Redmine ticket

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6346e02d-282e-4009-9321-238df7bd92df

📥 Commits

Reviewing files that changed from the base of the PR and between 80b9225 and dbbf5d0.

📒 Files selected for processing (2)

• src/cloudai/_core/installables.py
• tests/test_base_installer.py

---

📝 Walkthrough

Walkthrough

DockerImage.cache_filename now builds an intermediate path and returns path.replace("/", "_").replace("#", "_").strip("_") to more aggressively sanitize generated cache filenames; tests for cache_filename were extended with additional URL patterns.

Changes

Cohort / File(s)	Summary
Core Implementation src/cloudai/_core/installables.py	Adjusted DockerImage.cache_filename to use an intermediate path, replacing / and # with _ and trimming leading/trailing underscores before returning the cache filename.
Test Coverage tests/test_base_installer.py	Extended test_docker_cache_filename parameterization with additional Docker URL/path cases and updated expected sanitized .sqsh filenames.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I nibbled paths beneath the moon's bright beam,
Turned slashes into underscored seam by seam.
Sharp signs softened, edges trimmed just right,
Cache names tidy, hopping into night. 🥕✨

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly relates to the main change: preventing nested docker image paths by sanitizing cache filenames to ensure they are file names rather than paths.
Description check	✅ Passed	The description accurately explains the problem (nested paths from complex Docker URLs causing installation failures) and the solution (ensuring URLs resolve to filenames, not paths), which aligns with the code changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ipod/p1/docker-nested

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cloudai/_core/installables.py`:
- Around line 56-69: The docstring for the DockerImage.cache_filename property
is too concrete and duplicates test data; update the DockerImage.cache_filename
property docstring to a short high-level description of what the property
returns (e.g., "Return the cache filename for the docker image") and remove the
example matrix so behavior is documented by tests (leave behavioral examples in
the parametrized test instead). Ensure you only edit the docstring in the
cache_filename property of the DockerImage class.
- Around line 73-80: The bug is that tag splitting uses reference.rsplit(":", 1)
which can capture colons in parent directories (e.g., ./cache:tmp/image.sqsh)
and leave slashes in tag; instead parse the tag from the basename only: take the
basename of reference (or image_ref after initial assignment) and split that
basename on ":" to extract tag, then reconstruct image_ref as the original path
with the basename portion replaced by the pre-tag basename (so image_ref has no
trailing :tag) and set image_name from that pre-tag basename; update the code
around image_ref, tag, and image_name (the variables currently set in this
branch) so cache_filename / install_path use the cleaned image_name without any
slashes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e20e8381-78a0-481d-9c3f-04065ffa705b

📥 Commits

Reviewing files that changed from the base of the PR and between 6efd2be and 96de7fb.

📒 Files selected for processing (2)

• src/cloudai/_core/installables.py
• tests/test_base_installer.py

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

src/cloudai/_core/installables.py (1)

60-63: ⚠️ Potential issue | 🔴 Critical

Condition mismatch causes ValueError when : appears in directory but not filename.

Line 62 checks if ":" in reference: but the rsplit on line 63 operates on image_name. For a path like ./cache:tmp/file.sqsh:

• Path(reference).name → file.sqsh
• ":" in reference → True
• "file.sqsh".rsplit(":", 1) → ['file.sqsh'] (single element)
• Unpacking raises ValueError: not enough values to unpack

The condition should check the same variable that gets split.

🐛 Proposed fix

         if reference.startswith(("/", ".")):
             image_name = Path(reference).name
-            if ":" in reference:
+            if ":" in image_name:
                 image_name, tag = image_name.rsplit(":", maxsplit=1)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cloudai/_core/installables.py` around lines 60 - 63, The bug is that the
code checks ":" in reference but then splits image_name, causing a ValueError
when a ":" is in a directory component; update the logic in the block that
computes image_name/tag (the variables reference, image_name, tag and the
Path(reference).name use and the rsplit call) so the condition checks the same
variable being split (e.g., use if ":" in image_name) or use a safe split
(partition/rsplit and handle single-element result) before unpacking to ensure
tag is only extracted when present.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cloudai/_core/installables.py`:
- Around line 57-74: The cache filename change in cache_filename will break
existing caches; update the code and docs to preserve compatibility by having
DockerImageCacheManager (methods: cache_filename and uninstall_cached_image)
attempt legacy filename patterns when locating or deleting .sqsh files (e.g.,
try the new name first, then fallback to the previous naming scheme), and add a
short note in the release notes/docs explaining that old cache files will be
auto-detected or otherwise require cleanup so users are aware of potential
re-downloads or orphaned files.

---

Duplicate comments:
In `@src/cloudai/_core/installables.py`:
- Around line 60-63: The bug is that the code checks ":" in reference but then
splits image_name, causing a ValueError when a ":" is in a directory component;
update the logic in the block that computes image_name/tag (the variables
reference, image_name, tag and the Path(reference).name use and the rsplit call)
so the condition checks the same variable being split (e.g., use if ":" in
image_name) or use a safe split (partition/rsplit and handle single-element
result) before unpacking to ensure tag is only extracted when present.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2a6e36c4-a452-4f0e-b635-0425bb498620

📥 Commits

Reviewing files that changed from the base of the PR and between 96de7fb and 80b9225.

📒 Files selected for processing (1)

• src/cloudai/_core/installables.py