-
Notifications
You must be signed in to change notification settings - Fork 666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add manual_only
option to addon boot config
#5272
Conversation
WalkthroughWalkthroughThe changes introduce a new entity, Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this 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
Outside diff range, codebase verification and nitpick comments (1)
supervisor/addons/validate.py (1)
325-327
: Update docstring to reflect default value for ATTR_BOOT.The docstring should mention the default value for
ATTR_BOOT
to improve clarity.Update the docstring to include the default value:
""" Migrate addon config. - ATTR_BOOT: Default is AddonBootConfig.AUTO """
@@ -38,6 +38,7 @@ | |||
- This PR is related to issue: | |||
- Link to documentation pull request: | |||
- Link to cli pull request: | |||
- Link to client library pull request: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this makes sense, and ideally we also add a pull request template to the client library, which links back.
This is not really related to this PR, so it should be a separate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean if its important I can. Its pretty small though. And it is sort of related in that I want this PR to link to the Client library change. So I'd end up copying the new template into this PR description anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we squash commits on merge, this won't allow to revert the change later on. We don't revert that often, but that is the main reason why one change per PR is a thing.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
1793944
to
a5e89fa
Compare
There was a problem hiding this 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
Outside diff range, codebase verification and nitpick comments (1)
tests/fixtures/addons/local/example/config.yaml (1)
26-26
: Document themanual_only
configuration option.The
manual_only
configuration option is validated and tested in the codebase, as evidenced by its presence in several Python files. However, it is not documented in any Markdown files. Please ensure that this new configuration option is documented to provide clarity and guidance for its usage.
- Consider adding documentation for
manual_only
in relevant Markdown files.Analysis chain
LGTM! Verify documentation and validation.
The new configuration option
boot: manual_only
is correctly added. Ensure that this new option is documented and validated correctly in the codebase.The code changes are approved.
Run the following script to verify the documentation and validation of the new configuration option:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the documentation and validation of the new configuration option `boot: manual_only`. # Test: Search for the documentation of the new configuration option. Expect: Occurrences of `manual_only`. rg --type markdown 'manual_only' # Test: Search for the validation of the new configuration option. Expect: Occurrences of `manual_only`. rg --type python 'manual_only'Length of output: 94
Script:
#!/bin/bash # Description: Verify the documentation and validation of the new configuration option `boot: manual_only`. # Test: Search for the documentation of the new configuration option. Expect: Occurrences of `manual_only`. rg 'manual_only' --glob '*.md' # Test: Search for the validation of the new configuration option. Expect: Occurrences of `manual_only`. rg 'manual_only' --glob '*.py'Length of output: 598
manual_only
option to addon boot config
Proposed change
Some addons like the silabs flasher should never be set to auto-start under any circumstance. Add a
manual_only
option toboot
which allows addon developers to prevent this when the addon requires it.As this is the first PR that also requires client library changes to support it, updated the Pull Request template to mention that alongside CLI and API Doc.
Type of change
Additional information
Checklist
ruff format supervisor tests
)If API endpoints of add-on configuration are added/changed:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation