[wchilders-nvidia]

I ran into a problem where (I suspect) an agent triggered a long running task that generated lots of output and never terminated. This ended up filling a ~3 TB disk over the last couple of days.

The logs generated by these containers are equivalent to the shell history / this utility is not intended for logged background tasks in any case; so it seems the best option to prevent this from surprising anyone else is to simply disable all logging when launching these containers.

Summary by CodeRabbit

• Updates
  • Docker containers launched through the tool no longer have their output retained by Docker’s logging system. Output shown in the attached terminal remains available as before. This changes how container output is handled, without affecting the existing terminal display.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: a35313dc-e438-43b0-a5a7-7ed95f5910e4

Commits

Reviewing files that changed from the base of the PR and between 4cc57d4 and bda91c5.

Files selected for processing: 1

• dev_tools/pylibs/edgdocker/__init__.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Walkthrough

exec_docker_cmd now passes --log-driver none to docker run. Existing attached-terminal output handling remains in place.

Changes: Docker logging configuration

Layer / File(s)	Summary
Set the Docker log driver dev_tools/pylibs/edgdocker/__init__.py	exec_docker_cmd adds --log-driver none to the docker run arguments.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Suggested labels: infrastructure, bug

Suggested reviewers: daveedvdv-nvidia

Merge Risk: Minimal · up to bda91

The change disables persisted container logs while preserving live command output, with no identified repository workflow regression.

Security architecture risk: Low · up to bda91

The change prevents Docker-managed container logs from growing without bound, while retaining live command output and existing execution privileges. It also removes Docker log retention for all launches through the shared helper. No security dependency on those logs was established, but external logging requirements remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The effective change reaches containers launched through the shared helper on the selected Docker daemon. It overrides the daemon-default logging policy for those containers, rather than changing logging for unrelated containers.

Trust Boundaries and Controls

• inferred — The changed control is output retention, not caller authorization or container isolation. Attached output remains available to its existing recipient, but Docker-managed logging is no longer available as an independent record of that output.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly identifies the main change: disabling logging for edg-exec containers. The trailing brackets are unnecessary but do not prevent understanding.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[edgcpp-bot]

AcknowlEDG review results updated for bda91c5eb629:

• Bench - C (new)
• Bench - C++ (new)

CI success.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!