Convert qbittorrent to config flow #45618
Conversation
* update the qbittorrent api integration * Added flow * Added unittests * Removed the state attribute as suggested * Add Valid Credentials Test * Update tests/components/qbittorrent/test_config_flow.py Added the suggestions of ktnrg45 Co-authored-by: ktnrg45 <38207570+ktnrg45@users.noreply.github.com> * typo's * Fix Flake error * Added flow * Added unittests * Removed the state attribute as suggested * Add Valid Credentials Test * Update tests/components/qbittorrent/test_config_flow.py Added the suggestions of ktnrg45 Co-authored-by: ktnrg45 <38207570+ktnrg45@users.noreply.github.com> * typo's * Fix Flake error * Added flow * Added unittests * Removed the state attribute as suggested * Add Valid Credentials Test * Update tests/components/qbittorrent/test_config_flow.py Added the suggestions of ktnrg45 Co-authored-by: ktnrg45 <38207570+ktnrg45@users.noreply.github.com> * typo's * Fix Flake error * Added flow * Removed the state attribute as suggested Co-authored-by: ktnrg45 <38207570+ktnrg45@users.noreply.github.com>
|
sorry all for the inconvenience |
…qbittorentflow
|
fixed merge conflicts for when this branch gets merged into the upstream branch |
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
|
Should not be marked stale, still relevant. |
…qbittorentflow
…qbittorentflow
…qbittorentflow
…qbittorentflow
…qbittorentflow
|
Merged recent qbittorrent changes in this branch, PR is now open since 7 months ... |
…qbittorentflow
…qbittorentflow
…qbittorentflow
|
@frenck or @MartinHjelmare Do you guys want to take a look at this PR, please? |
…qbittorentflow
|
@cgarwood Maybe you want to take a look my PR? |
|
@MartinHjelmare please can you look at this, |
There was a problem hiding this comment.
Hi there @geoffreylagaisse 👋
I've added some initial review comments to get the review process going / rolling.
I would recommend you to git rebase this PR onto the latest dev branch before continuing development on this PR.
|
|
||
| async def async_setup(hass: HomeAssistant, config: dict): | ||
| """Set up the Qbittorrent component.""" | ||
| hass.data.setdefault(DOMAIN, {}) |
There was a problem hiding this comment.
This isn't used here, and should move to async_setup_entry
| from .client import create_client | ||
| from .const import DATA_KEY_CLIENT, DATA_KEY_NAME, DOMAIN | ||
|
|
||
| PLATFORMS = ["sensor"] |
There was a problem hiding this comment.
| PLATFORMS = ["sensor"] | |
| PLATFORMS = [Platform.SENSOR] |
| _LOGGER.error("Connection failed") | ||
| raise ConfigEntryNotReady from err |
There was a problem hiding this comment.
| _LOGGER.error("Connection failed") | |
| raise ConfigEntryNotReady from err | |
| raise ConfigEntryNotReady("Connection failed") from err |
|
|
||
| hass.data[DOMAIN][entry.data[CONF_URL]] = { | ||
| DATA_KEY_CLIENT: client, | ||
| DATA_KEY_NAME: name, |
There was a problem hiding this comment.
name seems to be a fixed value of "Qbittorrent", why would we need to add this to hass data?
| for platform in PLATFORMS: | ||
| hass.async_create_task( | ||
| hass.config_entries.async_forward_entry_setup(entry, platform) | ||
| ) | ||
|
|
There was a problem hiding this comment.
| for platform in PLATFORMS: | |
| hass.async_create_task( | |
| hass.config_entries.async_forward_entry_setup(entry, platform) | |
| ) | |
| hass.config_entries.async_setup_platforms(entry, PLATFORMS) |
| return | ||
| if self._attr_available: | ||
| _LOGGER.error("Connection lost") | ||
| self._attr_available = False | ||
| except self._exception: | ||
| _LOGGER.error("Invalid authentication") |
There was a problem hiding this comment.
| _LOGGER.error("Invalid authentication") | |
| _LOGGER.warning("Invalid authentication") |
| "config": { | ||
| "step": { | ||
| "user": { | ||
| "title": "qBitTorrent", |
There was a problem hiding this comment.
This doesn't need to be translated.
| "title": "qBitTorrent", |
|
|
||
| return self.async_abort(reason=errors["base"]) | ||
|
|
||
| async def async_step_import_confirm(self, user_input=None): |
There was a problem hiding this comment.
Don't use a confirmation step; instead, just import that data and migrate the user.
| }, | ||
| "import_confirm": { | ||
| "title": "Import qbittorret sensor", | ||
| "description": "A {id} sensor has been discovered in configuration.yaml. This flow will allow you to import it into a config entry." | ||
| } |
There was a problem hiding this comment.
This should not be needed and thus can be removed
| }, | |
| "import_confirm": { | |
| "title": "Import qbittorret sensor", | |
| "description": "A {id} sensor has been discovered in configuration.yaml. This flow will allow you to import it into a config entry." | |
| } | |
| } |
| }, | ||
| "abort": { | ||
| "already_configured": "[%key:common::config_flow::abort::already_configured_device%]", | ||
| "config_cannot_be_imported": "Config from configuration.yaml can not be imported" |
There was a problem hiding this comment.
I cannot find usages of this key?
frenck
added
the
config-flow
|
This PR seems to have gone stale. Will go ahead and close it for now. Feel free to pick it up / re-open it when ready to work on it again. Thanks for willing to contribute 👍 ../Frenck |
Proposed change
Upgrade the qbittorrent component to flow, and used the internal async processes of hass.
Fixes #48158
Type of change
Additional information
Checklist
black --fast 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..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: