Flasher driver: fix configuring with custom tftp/http driver#431
Flasher driver: fix configuring with custom tftp/http driver#431
Conversation
WalkthroughThe code in the driver module was modified to change how the Changes
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/driver.py (1)
38-47: Note: Consider improving the error message for consistency.The error messages for missing required children have a minor typo - "ir" should be "or".
- "'serial' instance is required for BaseFlasher either via a ref ir a direct child instance" + "'serial' instance is required for BaseFlasher either via a ref or a direct child instance" - "'power' instance is required for BaseFlasher either via a ref ir a direct child instance" + "'power' instance is required for BaseFlasher either via a ref or a direct child instance"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/driver.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: pytest-matrix (3.12)
- GitHub Check: e2e
- GitHub Check: pytest-matrix (3.11)
- GitHub Check: pytest-matrix (3.13)
- GitHub Check: build-and-push-image (jumpstarter-dev/jumpstarter-devspace .devfile/Containerfile.client)
- GitHub Check: build-and-push-image (jumpstarter-dev/jumpstarter-utils Dockerfile.utils)
🔇 Additional comments (2)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/driver.py (2)
30-32: Good fix: Unconditional assignment of TFTP driver ensures consistent behavior.The change ensures that
self.tftpis always assigned regardless of whether the TFTP driver was newly created or already existed inself.children. This fixes the ability to use custom TFTP drivers, as the previous implementation would only set the attribute when creating a default driver.
34-36: Good fix: Unconditional assignment of HTTP driver ensures consistent behavior.Similar to the TFTP change, moving the HTTP driver assignment outside the conditional block ensures that
self.httpis always set correctly. This ensures that the class works consistently with custom HTTP drivers specified in configuration.
|
Successfully created backport PR for |
Summary by CodeRabbit