Change qBittorrent lib to qbittorrentapi #113394
Conversation
|
Hey there @geoffreylagaisse, @finder39, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
Please don't merge dev into your branch United there is a need to |
Oh sorry, didn't mean to spam |
frenck
added
the
smash
|
No problem, just letting you know :) In the current case it doesn't really matter since the CI doesn't run automatically yet, but once you have a few PRs in, it will run the CI everytime, and whenever that's unneeded, we rather avoid that to prevent our CI from clogging up |
|
Sorry forgot to push the tests change, test fixed. |
|
Hey @finder39 , do you have some times to check this ? |
|
Sorry, just got back from a trip. Taking a look now! |
|
I tested locally (both as upgrade and fresh add of integration) and all seemed to work. I notice quite a few |
Yes the linter don't really like the typing using with this lib, and the pre-hook fail if you remove this I think. There is an issue open on the lib to fix that: rmartin16/qbittorrent-api#371 so hopefully we will be able to remove this in the future, I'll keep an eye on this |
|
Gotcha! Overall it looks good, so I'm good for this to be merged in. Definitely an upgrade since the old library hasn't been updated since early 2023 |
Nice ! Waiting for a reviewer with write access for this to be merged 😉 |
|
Did I need to ping someone for this to be reviewed ? I'm not familiar with big project so just tell me. |
|
I've already pinged someone to look earlier this week, they're just busy rn |
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Sebclem
force-pushed
the
feature/qbittorent_lib_change
branch
from
d83eeff to
0da9582
Compare
frenck
removed
the
smash
|
@edenhaus For all the |
| except Forbidden403Error as err: | ||
| raise ConfigEntryNotReady("Fail to log in, banned user ?") from err |
There was a problem hiding this comment.
Shouldn't this cause us to start a reauth flow instead? But maybe that's for a follow-up?
There was a problem hiding this comment.
Maybe in another PR because I need to take a better look to the documentation, as I have seen, we need to update the Config flow to make this work right ?
There was a problem hiding this comment.
Yes, that's right, the config flow needs to be updated. It's fine to do that in a separate PR.
Proposed change
python-qbittorrent python lib seams to not be actively maintained anymore.
This change aim to remove this lib and use a more actively maintained one: qbittorent-api
Github project: https://github.com/rmartin16/qbittorrent-api
PyPi: https://pypi.org/project/qbittorrent-api/
Doc: https://qbittorrent-api.readthedocs.io/en/latest/
This will also unlock #107637 as the implementation in incorrect in python-qbittorrent
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..coveragerc.To help with the load of incoming pull requests: