Skip to content

fix(clp-package): Use short-form docker stop timeout option to support older Docker versions (fixes #1322).#1323

Merged
kirkrodrigues merged 1 commit into
y-scope:mainfrom
kirkrodrigues:fix-1322
Sep 24, 2025
Merged

fix(clp-package): Use short-form docker stop timeout option to support older Docker versions (fixes #1322).#1323
kirkrodrigues merged 1 commit into
y-scope:mainfrom
kirkrodrigues:fix-1322

Conversation

@kirkrodrigues

@kirkrodrigues kirkrodrigues commented Sep 24, 2025 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Description

As the title says.

Docker renamed the --time option to --timeout in v28 which was only released in February. It seems unrealistic that most of our users would've upgraded to v28, so switching to the shortform option -t will allow greater compatibility.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

sbin/start-clp.sh
sbin/stop-clp.sh

Summary by CodeRabbit

  • Chores
    • Updated container stop behaviour to use the short-form timeout flag for improved compatibility across environments. The configured timeout value and overall behaviour remain unchanged.
    • No changes to user-facing commands or interfaces.
    • Improves consistency without affecting existing workflows.

@kirkrodrigues kirkrodrigues requested a review from a team as a code owner September 24, 2025 02:58
@coderabbitai

coderabbitai Bot commented Sep 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Walkthrough

Replaced the Docker stop flag in stop_running_container from the long form --timeout to the short form -t in stop_clp.py. No other logic, control flow, or public interfaces were changed.

Changes

Cohort / File(s) Summary
CLI stop script flag update
components/clp-package-utils/clp_package_utils/scripts/stop_clp.py		 Switched docker stop flag from --timeout to -t while keeping the same timeout value; no other changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related issues

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately describes the primary change: switching to Docker's short-form timeout option to maintain compatibility with older Docker releases. It follows conventional commit style with a clear scope ("clp-package") and includes the related issue number for traceability. The phrasing is specific, concise, and directly reflective of the changeset.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 17ec517 and 50ab7ef.

📒 Files selected for processing (1)
  • components/clp-package-utils/clp_package_utils/scripts/stop_clp.py (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: package-image
🔇 Additional comments (1)
components/clp-package-utils/clp_package_utils/scripts/stop_clp.py (1)

42-42: Switch to -t is correct for cross-version Docker compatibility.

Using -t maintains compatibility with pre‑v28 and v28+ Docker. No further changes needed here.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Ruff (0.13.1)
components/clp-package-utils/clp_package_utils/scripts/stop_clp.py

�[1;31mruff failed�[0m
�[1mCause:�[0m Failed to load configuration /ruff.toml
�[1mCause:�[0m Failed to parse /ruff.toml
�[1mCause:�[0m TOML parse error at line 26, column 3
|
26 | "RSE100", # Use of assert detected
| ^^^^^^^^
Unknown rule selector: RSE100

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

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

@kirkrodrigues kirkrodrigues merged commit ddfcc5f into y-scope:main Sep 24, 2025
21 checks passed
@kirkrodrigues kirkrodrigues deleted the fix-1322 branch September 24, 2025 03:47
junhaoliao pushed a commit to junhaoliao/clp that referenced this pull request May 17, 2026
@kirkrodrigues
fix(clp-package): Use short-form docker stop timeout option to supp…
5bf5cab 
…ort older Docker versions (fixes y-scope#1322). (y-scope#1323)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidlion davidlion davidlion approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kirkrodrigues @davidlion