Local Docker Image Fix

[KesterTan]

Tango job hangs indefinitely due because it does not recognize the name of docker images.

This PR implements Bryan Parno's fix.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request modifies docker command execution in vmms/localDocker.py. The getImages function's docker command output format is changed to display only the repository column, and the getPartialOutput function is augmented with a 30-second subprocess execution timeout.

Changes

Cohort / File(s)	Summary
Docker Command Execution vmms/localDocker.py	Modified getImages to use docker images --format 'table {{.Repository}}' instead of docker images, altering output format. Added 30-second timeout parameter to subprocess call in getPartialOutput function.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Local Docker Image Fix' is directly related to the changeset, which modifies docker image handling in the getImages function to fix how docker images are recognized.
Description check	✅ Passed	The description explains the problem (Tango jobs hanging due to docker image name recognition) and states the PR implements a fix for this issue, which aligns with the changes made to docker image parsing.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch docker-images-fix

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@vmms/localDocker.py`:
- Around line 284-286: The call to subprocess.check_output(..., timeout=30)
assigned to output can raise subprocess.TimeoutExpired; wrap that call in a
try/except that catches subprocess.TimeoutExpired and handles it gracefully
(e.g., log the timeout with context, include the cmd and timeout value, and
either raise a controlled exception or return a sensible error/result), ensuring
the handler uses the same cmd and timeout=30 information so callers don't get an
unhandled exception from subprocess.check_output.

🧹 Nitpick comments (1)

vmms/localDocker.py (1)

261-270: LGTM! The format change correctly extracts repository names.

The updated command fixes the image name recognition issue by outputting only the Repository column.

Optional simplification: Consider using --format '{{.Repository}}' (without table) to omit the header row entirely, which would simplify the parsing and make the subsequent pop() call unnecessary.

♻️ Optional: Simplified parsing without table header

-        cmd = "docker images --format 'table {{.Repository}}'"
+        cmd = "docker images --format '{{.Repository}}'"
         o = subprocess.check_output(cmd, shell=True).decode("utf-8")
         o_l = o.split("\n")
         o_l.pop()
-        o_l.reverse()
-        o_l.pop()
         for row in o_l:
-            row_l = row.split(" ")
-            result.add(re.sub(r".*/([^/]*)", r"\1", row_l[0]))
+            if row:  # skip empty lines
+                result.add(re.sub(r".*/([^/]*)", r"\1", row))
         return list(result)

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Handle subprocess.TimeoutExpired exception.

The timeout=30 parameter will cause subprocess.check_output to raise subprocess.TimeoutExpired if the command exceeds 30 seconds. Without handling this exception, the function will propagate an unhandled exception to callers, potentially causing unexpected failures.

🐛 Proposed fix to handle timeout exception

+        try:
             output = subprocess.check_output(
                 cmd, stderr=subprocess.STDOUT, shell=True, timeout=30
             ).decode("utf-8")
+        except subprocess.TimeoutExpired:
+            self.log.warning("Timeout getting partial output for %s" % instanceName)
+            output = ""
+        except subprocess.CalledProcessError as e:
+            self.log.warning("Failed to get partial output for %s: %s" % (instanceName, e))
+            output = ""

         return output

🧰 Tools

🪛 Ruff (0.14.11)

284-284: subprocess call with shell=True identified, security issue

(S602)

🤖 Prompt for AI Agents

In `@vmms/localDocker.py` around lines 284 - 286, The call to
subprocess.check_output(..., timeout=30) assigned to output can raise
subprocess.TimeoutExpired; wrap that call in a try/except that catches
subprocess.TimeoutExpired and handles it gracefully (e.g., log the timeout with
context, include the cmd and timeout value, and either raise a controlled
exception or return a sensible error/result), ensuring the handler uses the same
cmd and timeout=30 information so callers don't get an unhandled exception from
subprocess.check_output.