-
Notifications
You must be signed in to change notification settings - Fork 610
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
Create issue for detached addons #5084
Conversation
@agners I choose not to include the repo URL in stored addon data. Detached addons where the repo was removed generally shouldn't happen other then current users hit with bugs in the past (and that won't help them). Each addon includes a URL linking to where to find information for that particular addon. So I'm going to link to that in the repair and tell people to find more info there. That is available even for existing users in this situation. |
@property | ||
def states(self) -> list[CoreState]: | ||
"""Return a list of valid states when this check can run.""" | ||
return [CoreState.RUNNING, CoreState.SETUP] |
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.
Doesn't RUNNING
makes this a periodical check?
I think once at startup should be enough.
@property | ||
def states(self) -> list[CoreState]: | ||
"""Return a list of valid states when this check can run.""" | ||
return [CoreState.RUNNING, CoreState.SETUP] |
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.
return [CoreState.RUNNING, CoreState.SETUP] | |
return [CoreState.STARTUP, CoreState.RUNNING] |
At this point, addons are loaded, SETUP can be to early I feel. I guess we need RUNNING as well detect detach add-ons after store updates
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.
@pvizeli we can do either. Currently the healthcheck during SETUP
is at the very end after everything else in that step so all addon information is loaded:
Line 191 in 67ec89c
await self.sys_resolution.evaluate.evaluate_system() |
I just removed RUNNING
from there as per @agners request although I can add it back if we want. Once at startup of supervisor seemed like enough, this should be uncommon other then possibly the source code of a local add-on being removed so I don't think we need to catch this immediately.
ea67c36
to
67ec89c
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.
Okay, let's test that
Proposed change
Create an issue if we detect a detached addon so we can make this into a repair and inform users they aren't receiving updates for these addons.
Type of change
Additional information
Checklist
ruff format supervisor tests
)If API endpoints of add-on configuration are added/changed: