search

JohnDoee / deluge-client

A very lightweight pure-python Deluge RPC Client
MIT License
87 stars 14 forks source link

Add automatic login for local Deluge instances #36

Closed bozhodimitrov closed 4 years ago

bozhodimitrov commented 4 years ago

Encapsulates the logic for automatic login to local Deluge instance into separate LocalDelugeRPCClient class. This will allow easy maintenance/refactoring or removal in the future. Code was back-ported from CouchPotato/CouchPotatoServer/tree/master/libs/synchronousdeluge Made some changes: better error handling and support for Python 3.

What do you think? It is not messing up with the existing code base and can be axed at any point in the future. I can also add documentation in the README and additional test case if you want?

JohnDoee commented 4 years ago

The problem with CouchPotato is that it's GPL so can't use code directly from it as the deluge-client is MIT.

It's a big problem and I've written lots of code because a library author decided to go with GPL.

Encapsulating it is a good choice so it's easy to avoid magic.

bozhodimitrov commented 4 years ago

I modified most of the back-ported code and we are not using the whole library, but just one method. I think that the original author is @alshain (Christian Dale). Maybe we can ask him for permission?

JohnDoee commented 4 years ago

The only thing that really needs to be written from scratch if we ditch the currently sourced code is the auth file discovery.

It is probably easier just to write that from scratch.

Linux config file discovery code: https://github.com/JohnDoee/autotorrent/blob/develop/autotorrent/clients/deluge.py#L62-L78

Parsing code: https://github.com/JohnDoee/autotorrent/blob/develop/autotorrent/clients/deluge.py#L80-L99

Since that's my code it can just be copied with no new attributions.

Appdata for windows: https://stackoverflow.com/questions/13184414/how-can-i-get-the-path-to-the-appdata-directory-in-python - the config file can be found in %APPDATA%\deluge\

bozhodimitrov commented 4 years ago

Yea, I think that I am using this code already. The problem now is that for some reason the Linux Deluge instance returns Username does not exist and I think that it is encoding issue.

PS: Doh... this error has to do with the fact that the local deluge instance runs under different user...

bozhodimitrov commented 4 years ago

Ok, I think that this is it. I am using your code with some slight modifications. Btw, I skipped parsing the daemon port on purpose, because it can be passed to the constructor anyways. Less parsing sounds better to me in this case :)

Let me know if anything else is needed for this PR.

JohnDoee commented 4 years ago

Your parsing code ended up really concise.

One thing I'm a bit unsure about is how to use it correctly in a flow. Lets say I made some script that tries to use LocalDelugeRPCClient, two things can happen:

How would I use that correctly in a script? It's not immediately obvious to me but I feel like it should be.

bozhodimitrov commented 4 years ago

If you mean the error handling - It is the same as DelugeRPCClient. It will raise the same errors. I am not adding anything special.

If the _get_local_auth doesn't find any localclient creds, it will return empty username and password. And it will call DelugeRPCClient with that empty username and password - at this point, the error handling is in the hands of DelugeRPCClient.

I didn't add any additional error handling, because I wanted to preserve the default behavior of DelugeRPCClient. So if the local Deluge instance is not discovered - it will either raise ConnectionRefusedError (if it can't connect, because of wrong ip/port/firewall restriction) or it will raise exception with msg "Wrong username or password"/"Username does not exist"/etc.

Also keep in mind that this is only for default Deluge installations:

  1. Connection should be local (127.0.0.1/::1 or localhost for host).
  2. Auth file should be at the default location.
  3. Username localclient should exist - this username is the one that the Deluge GTK UI thin client uses itself to connect to its local deluged backend and is generated by default.

We can raise an error if the local username and password are not found, but I don't think that it will be a big enhancement. Let me know if you have any idea around that.

JohnDoee commented 4 years ago

Should be good enough, thanks for making it.

Anything else before I create a release ?

bozhodimitrov commented 4 years ago

Nope, thanks for adding this to the code base - I know that it is a small "feature" and it is not very important, but it really makes it very convenient for home automation purposes and local deployments.

And if you want something else around it, just let me know! Have a nice day and once again, thanks for making this project. 🍻