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 8 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
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,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
67 changes: 67 additions & 0 deletions homeassistant/components/qbittorrent/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1,68 @@
"""The qbittorrent component."""
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 PlatformNotReady
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
except RequestException as err:
_LOGGER.error("Connection failed")
raise PlatformNotReady 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
19 changes: 19 additions & 0 deletions homeassistant/components/qbittorrent/client.py
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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="config_cannot_be_imported")

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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
92 changes: 60 additions & 32 deletions homeassistant/components/qbittorrent/sensor.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Support for monitoring the qBittorrent API."""
import logging

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

Expand All @@ -14,17 +14,20 @@
DATA_RATE_KILOBYTES_PER_SECOND,
STATE_IDLE,
)
from homeassistant.exceptions import PlatformNotReady
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.entity import Entity

_LOGGER = logging.getLogger(__name__)

SENSOR_TYPE_CURRENT_STATUS = "current_status"
SENSOR_TYPE_DOWNLOAD_SPEED = "download_speed"
SENSOR_TYPE_UPLOAD_SPEED = "upload_speed"
from .client import get_main_data_client
from .const import (
DATA_KEY_CLIENT,
DATA_KEY_NAME,
DOMAIN,
SENSOR_TYPE_CURRENT_STATUS,
SENSOR_TYPE_DOWNLOAD_SPEED,
SENSOR_TYPE_UPLOAD_SPEED,
)

DEFAULT_NAME = "qBittorrent"
_LOGGER = logging.getLogger(__name__)

SENSOR_TYPES = {
SENSOR_TYPE_CURRENT_STATUS: ["Status", None],
Expand All @@ -37,32 +40,28 @@
vol.Required(CONF_URL): cv.url,
vol.Required(CONF_USERNAME): cv.string,
vol.Required(CONF_PASSWORD): cv.string,
vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
vol.Optional(CONF_NAME, default=DOMAIN): cv.string,
Copy link
Member

Choose a reason for hiding this comment

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

Does this change the behavior? Previously it was qBittorrent, now it becomes qbitorrent, is this change in behavior made for a reason?

}
)


def setup_platform(hass, config, add_entities, discovery_info=None):
"""Set up the qBittorrent sensors."""

try:
client = Client(config[CONF_URL])
client.login(config[CONF_USERNAME], config[CONF_PASSWORD])
except LoginRequired:
_LOGGER.error("Invalid authentication")
return
except RequestException as err:
_LOGGER.error("Connection failed")
raise PlatformNotReady from err

name = config.get(CONF_NAME)
async def async_setup_entry(hass, entry, async_add_entities):
"""Set up the qBittorrent sensor."""

dev = []
for sensor_type in SENSOR_TYPES:
sensor = QBittorrentSensor(sensor_type, client, name, LoginRequired)
dev.append(sensor)

add_entities(dev, True)
qbit_data = hass.data[DOMAIN][entry.data[CONF_URL]]
name = qbit_data[DATA_KEY_NAME]
variables = SENSOR_TYPES
sensors = [
QBittorrentSensor(
sensor_name,
qbit_data[DATA_KEY_CLIENT],
name,
LoginRequired,
entry.entry_id,
)
for sensor_name in variables
]
async_add_entities(sensors, True)


def format_speed(speed):
Expand All @@ -74,7 +73,14 @@ def format_speed(speed):
class QBittorrentSensor(Entity):
"""Representation of an qBittorrent sensor."""

def __init__(self, sensor_type, qbittorrent_client, client_name, exception):
def __init__(
self,
sensor_type,
qbittorrent_client,
client_name,
exception,
server_unique_id,
):
"""Initialize the qBittorrent sensor."""
self._name = SENSOR_TYPES[sensor_type][0]
self.client = qbittorrent_client
Expand All @@ -84,12 +90,18 @@ def __init__(self, sensor_type, qbittorrent_client, client_name, exception):
self._unit_of_measurement = SENSOR_TYPES[sensor_type][1]
self._available = False
self._exception = exception
self._server_unique_id = server_unique_id

@property
def name(self):
"""Return the name of the sensor."""
return f"{self.client_name} {self._name}"

@property
def unique_id(self):
"""Return the unique id of the sensor."""
return f"{self._server_unique_id}/{self._name}"

@property
def state(self):
"""Return the state of the sensor."""
Expand All @@ -105,10 +117,26 @@ def unit_of_measurement(self):
"""Return the unit of measurement of this entity, if any."""
return self._unit_of_measurement

def update(self):
@property
def icon(self):
"""Icon to use in the frontend, if any."""
return "mdi:cloud-download"
Comment on lines +131 to +134
Copy link
Member

Choose a reason for hiding this comment

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

The icon can be set in the entity description instead.


@property
def device_info(self):
"""Return the device information of the entity."""
return {
"identifiers": {(DOMAIN, self._server_unique_id)},
"name": self.client_name,
"manufacturer": "QBittorrent",
}

async def async_update(self):
"""Get the latest data from qBittorrent and updates the state."""
try:
data = self.client.sync_main_data()
data = await self.hass.async_add_executor_job(
get_main_data_client, self.client
)
Comment on lines +149 to +151
Copy link
Member

Choose a reason for hiding this comment

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

This will get the data for each sensor individually, you might want to consider implementing a DataUpdateCoordinator to handle the updates for all sensors centrally.

See: https://developers.home-assistant.io/docs/integration_fetching_data/#coordinated-single-api-poll-for-data-for-all-entities

self._available = True
except RequestException:
_LOGGER.error("Connection lost")
Expand Down