Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

tmt: remove default reboot_cmd which causes issues with some tmt versions#710

Merged
bennyz merged 1 commit intomainfrom
mangelajo-patch-1
Oct 16, 2025
Merged

tmt: remove default reboot_cmd which causes issues with some tmt versions#710
bennyz merged 1 commit intomainfrom
mangelajo-patch-1

Conversation

@mangelajo
Copy link
Copy Markdown
Member

@mangelajo mangelajo commented Oct 16, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Changed the TMT driver's default reboot command from a preset value to empty. Users relying on the previous default should explicitly configure their preferred reboot command to restore prior behavior.

@netlify
Copy link
Copy Markdown

netlify Bot commented Oct 16, 2025

Deploy Preview for jumpstarter-docs ready!

Name Link
🔨 Latest commit dfd52bc
🔍 Latest deploy log https://app.netlify.com/projects/jumpstarter-docs/deploys/68f0c519485ecd0008946718
😎 Deploy Preview https://deploy-preview-710--jumpstarter-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 16, 2025

Walkthrough

The TMT driver's dataclass default for the reboot_cmd field was changed from "j power cycle" to an empty string (""), and corresponding tests were updated to expect the new default. No other logic or methods were modified.

Changes

Cohort / File(s) Change Summary
TMT Driver Source
packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/driver.py
Changed TMT.reboot_cmd default from "j power cycle" to "".
TMT Driver Tests
packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/driver_test.py
Updated test expectations to reflect reboot_cmd default now being an empty string.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A quiet reboot, hushed and small,
The old command folded, now nothing at all.
I hop on tiptoe, nibble the change,
Empty string settles—soft and strange. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly states the removal of the default reboot_cmd in the TMT driver and identifies the motivating issue with certain TMT versions, directly reflecting the change in the driver API. It concisely summarizes the primary update without unnecessary detail, giving reviewers immediate insight into the purpose of the change.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ 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 mangelajo-patch-1

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.

@mangelajo mangelajo changed the title tmt remvoe default reboot_cmd which causes issues with some tmt versions tmt: remove default reboot_cmd which causes issues with some tmt versions Oct 16, 2025
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7890749 and 54c2b9e.

📒 Files selected for processing (1)
  • packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/driver.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). (11)
  • GitHub Check: Redirect rules - jumpstarter-docs
  • GitHub Check: Header rules - jumpstarter-docs
  • GitHub Check: Pages changed - jumpstarter-docs
  • GitHub Check: build
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
  • GitHub Check: pytest-matrix (macos-15, 3.11)
  • GitHub Check: pytest-matrix (macos-15, 3.13)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
  • GitHub Check: pytest-matrix (macos-15, 3.12)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
  • GitHub Check: e2e

Comment thread packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/driver.py
@mangelajo mangelajo requested a review from bennyz October 16, 2025 10:15
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/driver.py (1)

11-11: Add docstring clarifying empty reboot_cmd semantics.

Tests have been updated to reflect the new default (good!), but the TMT dataclass still lacks documentation explaining what an empty reboot_cmd means. Based on the test test_replace_provision_args_without_hard_reboot in driver_test.py, it appears an empty string skips the --hard-reboot flag in TMT provisioning.

Consider adding a docstring or inline comment:

 @dataclass(kw_only=True)
 class TMT(Driver):
-    """ driver for Jumpstarter"""
+    """Driver for Jumpstarter TMT integration.
+    
+    Attributes:
+        reboot_cmd: Command for hard reboots during TMT provisioning.
+                    Empty string (default) omits --hard-reboot flag.
+        default_username: Default SSH username for TMT connections.
+        default_password: Default SSH password for TMT connections.
+    """
 
     reboot_cmd: str = ""
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54c2b9e and dfd52bc.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/driver.py (1 hunks)
  • packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/driver_test.py (4 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). (11)
  • GitHub Check: Redirect rules - jumpstarter-docs
  • GitHub Check: Header rules - jumpstarter-docs
  • GitHub Check: Pages changed - jumpstarter-docs
  • GitHub Check: build
  • GitHub Check: pytest-matrix (macos-15, 3.13)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
  • GitHub Check: pytest-matrix (macos-15, 3.12)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
  • GitHub Check: pytest-matrix (macos-15, 3.11)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
  • GitHub Check: e2e
🔇 Additional comments (2)
packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/driver_test.py (2)

137-137: LGTM! Test assertions updated correctly.

These mock call assertions now correctly expect the new default reboot_cmd value of "" instead of "j power cycle". The changes align with the driver.py update.

Also applies to: 151-151, 171-171


239-239: LGTM! Default value test updated correctly.

The test now correctly asserts the new default reboot_cmd value of "". This aligns with the driver.py change.

@bennyz bennyz merged commit 3080057 into main Oct 16, 2025
18 checks passed
@bennyz bennyz deleted the mangelajo-patch-1 branch October 16, 2025 10:34
@jumpstarter-backport-bot
Copy link
Copy Markdown

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants