Skip to content
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

Convert qbittorrent to config flow #45618

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
82b9467
Feature/qbittorrentconfigflow (#5)
geoffreylagaisse Jan 27, 2021
c20c9d1
AddMissingLines
geoffreylagaisse Jan 27, 2021
f06a65d
Resolve feedback
geoffreylagaisse Jan 31, 2021
f4ebb14
Resolve feedback - add Migrate flow
geoffreylagaisse Jan 31, 2021
d763c99
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse Feb 1, 2021
33ff29a
resolve some feedback
geoffreylagaisse Feb 1, 2021
1d49571
do the Isort
geoffreylagaisse Feb 1, 2021
b97ec4e
Pix Pylint error
geoffreylagaisse Feb 1, 2021
e0ca532
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse Feb 17, 2021
3a30993
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse Mar 20, 2021
d066afb
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse Apr 14, 2021
3d357ae
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse Apr 24, 2021
71a2010
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse May 1, 2021
7d3ddd7
Stop flooding the logs with error messages
geoffreylagaisse May 1, 2021
912a8cc
Added extra tests
geoffreylagaisse May 1, 2021
aad410f
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse May 23, 2021
26aea2b
added unload
geoffreylagaisse May 23, 2021
a97ac11
Added import init test
geoffreylagaisse May 24, 2021
7bd7ff9
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse Jun 13, 2021
99a0c8d
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse Jul 3, 2021
fbfaf71
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse Aug 17, 2021
9d15866
Fix Pyupgrade error, rest I fix after my holiday
geoffreylagaisse Aug 17, 2021
fcab3b3
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse Sep 2, 2021
c372bac
Fixes after merging latest changes
geoffreylagaisse Sep 2, 2021
65907f0
added extra test
geoffreylagaisse Sep 5, 2021
5bbfe42
assert zero for faulty test
geoffreylagaisse Sep 5, 2021
35bde58
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse Sep 23, 2021
b7c78f8
Added new test and better error handling
geoffreylagaisse Sep 23, 2021
52c0bfe
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse Sep 24, 2021
b61fa70
Merge branch 'dev' of https://github.com/home-assistant/core into add…
geoffreylagaisse Oct 17, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .coveragerc
Expand Up @@ -839,6 +839,7 @@ omit =
homeassistant/components/pvoutput/sensor.py
homeassistant/components/pyload/sensor.py
homeassistant/components/qbittorrent/sensor.py
homeassistant/components/qbittorrent/client.py
homeassistant/components/qnap/sensor.py
homeassistant/components/qrcode/image_processing.py
homeassistant/components/quantum_gateway/device_tracker.py
Expand Down
84 changes: 84 additions & 0 deletions homeassistant/components/qbittorrent/__init__.py
@@ -1 +1,85 @@
"""The qbittorrent component."""
import asyncio
import logging

from qbittorrent.client import LoginRequired
from requests.exceptions import RequestException

from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry
from homeassistant.const import CONF_PASSWORD, CONF_URL, CONF_USERNAME
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryNotReady
from homeassistant.helpers import config_per_platform

from .client import create_client
from .const import DATA_KEY_CLIENT, DATA_KEY_NAME, DOMAIN

PLATFORMS = ["sensor"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
PLATFORMS = ["sensor"]
PLATFORMS = [Platform.SENSOR]


_LOGGER = logging.getLogger(__name__)


async def async_setup(hass: HomeAssistant, config: dict):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this logic is done inside sensor.py, you can removed the part of needing the handle the config per platform and trigger the import in async_setup_platform of the sensor instead.

(Which is also a nice place to throw a deprecation warning of the YAML configuration being deprecated).

"""Set up the Qbittorrent component."""
hass.data.setdefault(DOMAIN, {})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't used here, and should move to async_setup_entry


# Import configuration from sensor platform
config_platform = config_per_platform(config, "sensor")
for p_type, p_config in config_platform:
if p_type != DOMAIN:
continue

hass.async_create_task(
hass.config_entries.flow.async_init(
DOMAIN,
context={"source": SOURCE_IMPORT},
data=p_config,
)
)

return True


async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry):
async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:

"""Set up Qbittorrent from a config entry."""
name = "Qbittorrent"
try:
client = await hass.async_add_executor_job(
create_client,
entry.data[CONF_URL],
entry.data[CONF_USERNAME],
entry.data[CONF_PASSWORD],
)
except LoginRequired:
_LOGGER.error("Invalid authentication")
return False

except RequestException as err:
_LOGGER.error("Connection failed")
raise ConfigEntryNotReady from err
Comment on lines +58 to +59
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_LOGGER.error("Connection failed")
raise ConfigEntryNotReady from err
raise ConfigEntryNotReady("Connection failed") from err


hass.data[DOMAIN][entry.data[CONF_URL]] = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the entry_id as the index

Suggested change
hass.data[DOMAIN][entry.data[CONF_URL]] = {
hass.data[DOMAIN][entry.entry_id] = {

DATA_KEY_CLIENT: client,
DATA_KEY_NAME: name,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
)

Comment on lines +65 to +69
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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 True


async def async_unload_entry(hass: HomeAssistant, config_entry: ConfigEntry):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
async def async_unload_entry(hass: HomeAssistant, config_entry: ConfigEntry):
async def async_unload_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool:

"""Unload Qbittorrent Entry from config_entry."""

unload_ok = all(
await asyncio.gather(
*(
hass.config_entries.async_forward_entry_unload(config_entry, platform)
for platform in PLATFORMS
)
)
)

return unload_ok
Comment on lines +75 to +85
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unload_ok = all(
await asyncio.gather(
*(
hass.config_entries.async_forward_entry_unload(config_entry, platform)
for platform in PLATFORMS
)
)
)
return unload_ok
unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
if unload_ok:
del hass.data[DOMAIN][entry.entry_id]
return unload_ok

19 changes: 19 additions & 0 deletions homeassistant/components/qbittorrent/client.py
@@ -0,0 +1,19 @@
"""Some common functions used in the qbittorrent components."""
from qbittorrent.client import Client


def get_main_data_client(client: Client):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing return type

"""Get the main data from the Qtorrent client."""
return client.sync_main_data()
Comment on lines +5 to +7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the benefit of having this in a separate method?



def create_client(url, username, password):
"""Create the Qtorrent client."""
client = Client(url)
client.login(username, password)
return client
Comment on lines +10 to +14
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used in a single place. I think it would make the code more readable if this was just moved into the async_setup_entry method directly.



def retrieve_torrentdata(client: Client, torrentfilter):
"""Retrieve torrent data from the Qtorrent client with specific filters."""
return client.torrents(filter=torrentfilter)
Comment on lines +17 to +19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a proxy method, is this really useful?
As in, we can just call the thing directly in the places that use it?

It is also partly typed, please type methods fully.

116 changes: 116 additions & 0 deletions homeassistant/components/qbittorrent/config_flow.py
@@ -0,0 +1,116 @@
"""Config flow for qBittorrent integration."""
import logging

from qbittorrent.client import LoginRequired
from requests.exceptions import RequestException
import voluptuous as vol

from homeassistant import config_entries
from homeassistant.const import CONF_PASSWORD, CONF_URL, CONF_USERNAME

from .client import create_client
from .const import DOMAIN # pylint:disable=unused-import
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unneeded disable

Suggested change
from .const import DOMAIN # pylint:disable=unused-import
from .const import DOMAIN


_LOGGER = logging.getLogger(__name__)


async def validate_input(hass, data):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we type this?

"""Validate user or import input."""
errors = {}
try:
await hass.async_add_executor_job(
create_client,
data[CONF_URL],
data[CONF_USERNAME],
data[CONF_PASSWORD],
)
except LoginRequired:
errors["base"] = "invalid_auth"

except RequestException as err:
errors["base"] = "cannot_connect"
_LOGGER.error("Connection failed - %s", err)
return errors


class QBittorrentConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
"""Handle a config flow for qbittorrent."""

VERSION = 1
CONNECTION_CLASS = config_entries.CONN_CLASS_CLOUD_POLL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer used

Suggested change
CONNECTION_CLASS = config_entries.CONN_CLASS_CLOUD_POLL


def __init__(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def __init__(self):
def __init__(self) -> None:

"""Initialize qbittorrent config flow."""
self._data = None
self._title = None

async def async_step_import(self, device_config):
"""Import a configuration.yaml config."""
data = {}

data[CONF_URL] = device_config.get(CONF_URL)
data[CONF_USERNAME] = device_config.get(CONF_USERNAME)
data[CONF_PASSWORD] = device_config.get(CONF_PASSWORD)

errors = await validate_input(self.hass, data)
if not errors:
self._data = data
self._title = data[CONF_URL]
return await self.async_step_import_confirm()
geoffreylagaisse marked this conversation as resolved.
Show resolved Hide resolved

return self.async_abort(reason=errors["base"])

async def async_step_import_confirm(self, user_input=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use a confirmation step; instead, just import that data and migrate the user.

"""Confirm the user wants to import the config entry."""
if user_input is None:
return self.async_show_form(
step_id="import_confirm",
description_placeholders={"id": self._title},
)

await self.async_set_unique_id(self._data[CONF_URL])
self._abort_if_unique_id_configured()

return self.async_create_entry(
title=self._data[CONF_URL],
data={
CONF_USERNAME: self._data[CONF_USERNAME],
CONF_PASSWORD: self._data[CONF_PASSWORD],
CONF_URL: self._data[CONF_URL],
},
)

async def async_step_user(self, user_input=None):
"""Handle the initial step."""
errors = {}
if user_input is not None:
await self.async_set_unique_id(user_input[CONF_URL])
self._abort_if_unique_id_configured()

_LOGGER.debug(
"Configuring user: %s - Password hidden", user_input[CONF_USERNAME]
)

errors = await validate_input(self.hass, user_input)

if not errors:
return self.async_create_entry(
title=user_input[CONF_URL],
data={
CONF_USERNAME: user_input[CONF_USERNAME],
CONF_PASSWORD: user_input[CONF_PASSWORD],
CONF_URL: user_input[CONF_URL],
},
)

return self.async_show_form(
step_id="user",
data_schema=vol.Schema(
{
vol.Required(CONF_URL): str,
vol.Required(CONF_USERNAME): str,
vol.Required(CONF_PASSWORD): str,
}
),
errors=errors,
)
25 changes: 25 additions & 0 deletions homeassistant/components/qbittorrent/const.py
@@ -0,0 +1,25 @@
"""Added constants for the qbittorrent component."""

SENSOR_TYPE_CURRENT_STATUS = "current_status"
SENSOR_TYPE_DOWNLOAD_SPEED = "download_speed"
SENSOR_TYPE_UPLOAD_SPEED = "upload_speed"
SENSOR_TYPE_TOTAL_TORRENTS = "total_torrents"
SENSOR_TYPE_ACTIVE_TORRENTS = "active_torrents"
SENSOR_TYPE_INACTIVE_TORRENTS = "inactive_torrents"
SENSOR_TYPE_DOWNLOADING_TORRENTS = "downloading_torrents"
SENSOR_TYPE_SEEDING_TORRENTS = "seeding_torrents"
SENSOR_TYPE_RESUMED_TORRENTS = "resumed_torrents"
SENSOR_TYPE_PAUSED_TORRENTS = "paused_torrents"
SENSOR_TYPE_COMPLETED_TORRENTS = "completed_torrents"

DEFAULT_NAME = "qbittorrent"
TRIM_SIZE = 35
CONF_CATEGORIES = "categories"

DOMAIN = DEFAULT_NAME

DATA_KEY_CLIENT = "client"
DATA_KEY_NAME = "name"

SERVICE_ADD_DOWNLOAD = "add_download"
SERVICE_REMOVE_DOWNLOAD = "remove_download"
1 change: 1 addition & 0 deletions homeassistant/components/qbittorrent/manifest.json
@@ -1,6 +1,7 @@
{
"domain": "qbittorrent",
"name": "qBittorrent",
"config_flow": true,
"documentation": "https://www.home-assistant.io/integrations/qbittorrent",
"requirements": ["python-qbittorrent==0.4.2"],
"codeowners": ["@geoffreylagaisse"],
Expand Down