JohnDoee / deluge-client

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

Add support for autodetecting/communicating with new deluge 2 protocol #29

Closed gazpachoking closed 5 years ago

gazpachoking commented 5 years ago

As described in #27, there were changes to the deluge 2 protocol recently. This PR adds support for those changes, and adds autodetection support for that version while retaining both deluge 1 and the unreleased deluge 2 protocol support.

The new protocol now has the protocol version in the header, which should make implementing any new changes in protocol and autodetecting them nicer.

I was unsure if we should still support the unreleased deluge 2 protocol which is now gone, but since my server is still running it, I left it in for now. 😉

I haven't added anything to the tests, this means a couple things:

fixes #27

gazpachoking commented 5 years ago

Looks like the linux tests are failing on deluge 2 because the twisted version it installs isn't new enough. I'll try to see if there is a PPA we can grab it from.

gazpachoking commented 5 years ago

Hmm. Unclear what I need to do to solve deluge 2 on linux tests. I'll try some more tomorrow.

JohnDoee commented 5 years ago

I didn't notice they already merged it when you posted the original issue.

The "original" Deluge 2 package is gone from their PPA and it seems like an awful lot of work to work around these issues. Testing against the latest official release is probably good enough (as it's already doing).

While this is a bug related to Deluge 2, it might be easier just to move everything to appveyor as they support Ubuntu 18.04?

gazpachoking commented 5 years ago

Hmm. Getting closer on enabling linux appveyor support, managed to get it to pass on deluge1/python27, but it seems to time out on the deluge 2 test with no info about what went wrong. Taking a break for now.

JohnDoee commented 5 years ago

I think it's related to Deluge 2 and the data it sends.

while True:
    try:
        d = self._socket.recv(READ_SIZE) # Stalls here first time it's called
    except ssl.SSLError:
        raise CallTimeoutException()

Even with a READ_SIZE of 1, it stalls so no data seems to be sent by Deluge.

gazpachoking commented 5 years ago

Hmm, I'll have to look closer. I ran the code against all 3 deluge versions locally and it was working on all 3. What kind of test were you doing to cause that?

JohnDoee commented 5 years ago

I SSHed the appveyor test server and just messed around with the code until the tests stalled - recv is a really annoying function as it'll stall forever.

gazpachoking commented 5 years ago

I think I may be on to something. Everything is working fine when run from windows, but I just tried running via WSL and it's detecting deluge version 1 then hanging.

gazpachoking commented 5 years ago

I think I've got it this time. Wasn't a windows/linux thing, it was that all my testing was on python 3 locally. Had to make a few tweaks to the code to make it work consistently on python 2 and 3. 🤞 appveyor passes this time.

gazpachoking commented 5 years ago

It's getting close now. Looks like on linux it's not actually testing on python 3 yet though.

kslr commented 5 years ago

There are some serious problems in Python3. Rpc is still normal

gazpachoking commented 5 years ago

It really seems like this should be installing pytest on python 3.6, I'm not sure why it isn't.

python --version
Python 3.6.7
sudo python -m pip install pytest
The directory '/home/appveyor/.cache/pip/http' or its parent directory is not owned by the current user and the cache has been disabled. Please check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag.
The directory '/home/appveyor/.cache/pip' or its parent directory is not owned by the current user and caching wheels has been disabled. check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag.
Collecting pytest
  Downloading https://files.pythonhosted.org/packages/95/6a/4122affe57eb3857874ef959a73f362e51b23812b96c92e073b16e1effd0/pytest-4.1.0-py2.py3-none-any.whl (215kB)
    100% |████████████████████████████████| 225kB 9.5MB/s 
Requirement already satisfied: setuptools in /usr/lib/python2.7/dist-packages (from pytest) (39.0.1)
gazpachoking commented 5 years ago

There are some serious problems in Python3. Rpc is still normal

What exactly do you mean @kslr ?

gazpachoking commented 5 years ago

Okay, I think we are there on appveyor tests now. /wipesbrow

JohnDoee commented 5 years ago

Good job :+1:

Thanks!