Fix hardcoded exception strings in uptimerobot#171744
Conversation
|
Hey there @ludeeus, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
This PR updates the UptimeRobot integration to use translated exception metadata (via translation_domain / translation_key) instead of hardcoded exception strings, and adjusts tests accordingly.
Changes:
- Add new exception message keys under
exceptionsinhomeassistant/components/uptimerobot/strings.json. - Raise
ConfigEntryAuthFailed/UpdateFailedusing translation metadata in the coordinator and setup entry. - Update the setup/reauth test to assert the new (translated) config entry failure reason.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
homeassistant/components/uptimerobot/coordinator.py |
Switches auth/update exceptions to translated exceptions; currently introduces a misleading translation key/placeholder for update failures. |
homeassistant/components/uptimerobot/__init__.py |
Switches wrong-API-key-type setup failure to translated exception metadata. |
homeassistant/components/uptimerobot/strings.json |
Adds new exceptions.* messages used by the translated exceptions. |
tests/components/uptimerobot/test_init.py |
Updates assertions to match the new auth-failure reason text. |
| raise UpdateFailed( | ||
| translation_domain=DOMAIN, | ||
| translation_key="api_exception", | ||
| translation_placeholders={"error": "Generic UptimeRobot exception"}, | ||
| ) from exception |
joostlek
left a comment
There was a problem hiding this comment.
We can remove the pylint exceptions
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
| raise UpdateFailed(exception) from exception | ||
| raise UpdateFailed( | ||
| translation_domain=DOMAIN, | ||
| translation_key="api_exception", |
There was a problem hiding this comment.
The string for "api_exception" is specific for turning on/off monitoring; it is not suitable for use here.
| translation_domain=DOMAIN, | ||
| translation_key="api_exception", | ||
| translation_key="api_switch_exception", | ||
| translation_placeholders={"error": "Generic UptimeRobot exception"}, |
| raise UpdateFailed( | ||
| translation_domain=DOMAIN, | ||
| translation_key="api_generic_exception", | ||
| translation_placeholders={"error": "Generic UptimeRobot exception"}, |
| assert mock_config_entry.state is ConfigEntryState.SETUP_ERROR | ||
| assert mock_config_entry.reason == "could not authenticate" | ||
| assert ( | ||
| mock_config_entry.reason | ||
| == "API authentication failed, please check your API key" | ||
| ) |
| assert exc_info.value.translation_domain == "uptimerobot" | ||
| assert exc_info.value.translation_key == "api_exception" | ||
| assert exc_info.value.translation_key == "api_switch_exception" | ||
| assert exc_info.value.translation_placeholders == { | ||
| "error": "Generic UptimeRobot exception" | ||
| } |
| assert exc_info.value.translation_domain == "uptimerobot" | ||
| assert exc_info.value.translation_key == "api_exception" | ||
| assert exc_info.value.translation_key == "api_switch_exception" | ||
| assert exc_info.value.translation_placeholders == { | ||
| "error": "Generic UptimeRobot exception" | ||
| } |
Proposed change
Fix hardcoded exception strings in uptimerobot, found in home-assistant/epics#71
Note: Used the same message already used in the integration. Will need a tweak in future: home-assistant/architecture#1397
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: