Blow up startup if init auth providers or modules failed #16240
Conversation
ghost
assigned 
| }) | ||
| assert provider is None | ||
| with pytest.raises(vol.Invalid): | ||
| provider = await auth_provider_from_config(hass, None, { |
There was a problem hiding this comment.
local variable 'provider' is assigned to but never used
|
I don't think we should do that. Because in unattended system it is ugly |
|
Failed load auth provider will leave system unprotected. Have discussion regarding this in #beta channel |
|
An alternative is that when the config is invalid, we default to the default config and issue a warning? That way the system is still accessible? |
|
Actually, we should not load a single config entry when auth is invalid, start in a "light" mode would be the best |
|
Was talking with Pascal: if config validation passes, HASS should successfully startup. If not, it's ok to fail. |
|
Double confirm my understanding is right, in try:
await async_process_ha_core_config();
except vol.Invalid:
return None # startup will hang
except HomeAssistantError: # such as import error
_LOGGER.error()
return None # (Solution A) startup will hang
or
return hass # (Solution B) a minimal HASS
or
pass # (Solution C) a HASS without auth protectionSo which solution is our final decision? |
tests/test_config.py
Outdated
| @@ -901,3 +901,20 @@ def test_merge_customize(hass): | |||
| } | |||
| with pytest.raises(Invalid): | |||
| await config_util.async_process_ha_core_config(hass, core_config) | |||
|
|
|||
|
|
|||
| async def test_disallowed_auth_provider_config(hass): | |||
There was a problem hiding this comment.
redefinition of unused 'test_disallowed_auth_provider_config' from line 889
tests/auth/test_init.py
Outdated
|
|
||
| from homeassistant import auth, data_entry_flow | ||
| from homeassistant.auth import ( | ||
| models as auth_models, auth_store, const as auth_const) | ||
| from homeassistant.auth.mfa_modules import SESSION_EXPIRATION | ||
| from homeassistant.exceptions import HomeAssistantError |
There was a problem hiding this comment.
'homeassistant.exceptions.HomeAssistantError' imported but unused
awarecan
changed the title
awarecan
changed the title
|
Have moved duplicate auth provider check to schema, so all config issue will raise vol.Invalid. I implemented as Solution A right now, startup will hang if load auth provider/mfa module failed. But can easily change to Solution B or C. |
* Blow up startup if init auth providers or modules failed * Delete core.entity_registry
…ant#16240) * Blow up startup if init auth providers or modules failed * Delete core.entity_registry
Description:
Blow up startup if init auth providers or modules failed, either import module error or config error
Related issue (if applicable): fixes #
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>
Example entry for
configuration.yaml(if applicable):Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
REQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices: