jaedb / Iris

Discover, explore and manage your music library across multiple sources with this beautiful web-based interface. Iris is a Mopidy frontend extension.
Apache License 2.0
1.14k stars 132 forks source link

Build for Python 3 #356

Closed jaedb closed 4 years ago

jaedb commented 5 years ago

Upgrade all python code to 3, with backwards compatibility to 2.7 (which is what Mopidy currently supports).

UPDATE: The Iris Python3 version will not be backwards compatible to Python 2.7 (as is the case with Mopidy 3).

kingosticks commented 4 years ago

Mopidy 3.0.0a3 is now on PyPI, this pre-release removes support for Python 2.7 and runs under Python 3.7 and 3.8. See https://github.com/mopidy/mopidy/issues/779#issuecomment-552116742

jodal commented 4 years ago

We're targeting a Mopidy 3 release Dec 21. Do you think Iris will have a working pre-release out by then?

jaedb commented 4 years ago

Unfortunately that's not looking likely. This is my only (and first!) Python project so it will take me a bit of time to work through the upgrade. I have started but progress is slow at this stage. Completion late January would be more realistic, also given the summer holiday period kicks off next week in New Zealand.

kingosticks commented 4 years ago

This porting checklist saves a lot of time, especially when you don't have much experience with Python 3 (like me).

kingosticks commented 4 years ago

Would it be helpful if I ran through the above checklist (as much as I can) and submitted a PR?

jaedb commented 4 years ago

@kingosticks I am grateful for any assistance, so yes please :-)

jaedb commented 4 years ago

I'm trying to install Mopidy 3.0.0-b1 but it's not available on pip for some reason, and running the mentioned python3 -m pip install --user Mopidy --pre installs version 2.1. What's the best way to install the Python3 version of Mopidy?

 python3 -m pip install --user Mopidy==3.0.0b1
Collecting Mopidy==3.0.0b1
  Could not find a version that satisfies the requirement Mopidy==3.0.0b1 (from versions: 0.1.0a0, 0.1.0a1, 0.1.0a2, 0.1.0a3, 0.1.0, 0.2.0, 0.2.1, 0.3.0, 0.3.1, 0.4.0, 0.4.1, 0.5.0, 0.6.0, 0.6.1, 0.7.0, 0.7.1, 0.7.2, 0.7.3, 0.8.0, 0.8.1, 0.9.0, 0.10.0, 0.11.0, 0.11.1, 0.12.0, 0.13.0, 0.14.0, 0.14.1, 0.14.2, 0.15.0, 0.16.0, 0.16.1, 0.17.0, 0.18.0, 0.18.1, 0.18.2, 0.18.3, 0.19.0, 0.19.1, 0.19.2, 0.19.3, 0.19.4, 0.19.5, 1.0.0, 1.0.1, 1.0.2, 1.0.3, 1.0.4, 1.0.5, 1.0.6, 1.0.7, 1.0.8, 1.1.0, 1.1.1, 1.1.2, 2.0.0, 2.0.1, 2.1.0)
No matching distribution found for Mopidy==3.0.0b1
kingosticks commented 4 years ago

I just installed it into a fresh python3.7 virtualenv using python3 -m pip install --user Mopidy==3.0.0b1. What version of Python 3 do you have here?

You can also install from source by following the instructions at https://docs.mopidy.com/en/develop/devenv/

jaedb commented 4 years ago

Hmmm seems peculiar. Ideally I'd run this in Docker but that can wait until I get the basics sorted.

python3 --version
Python 3.6.9
kingosticks commented 4 years ago

Mopidy 3 requires Python 3.7

jaedb commented 4 years ago

Mopidy should work on 3.7.6 then right?

root@d08d97de611f:/# mopidy

ERROR: A GObject based library was not found.

Mopidy requires GStreamer to work. GStreamer is a C library with a
number of dependencies itself, and cannot be installed with the regular
Python tools like pip.

Please see http://docs.mopidy.com/en/latest/installation/ for
instructions on how to install the required dependencies.

Traceback (most recent call last):
  File "/usr/local/bin/mopidy", line 5, in <module>
    from mopidy.__main__ import main
  File "/usr/local/lib/python3.7/site-packages/mopidy/__main__.py", line 7, in <module>
    from mopidy import commands
  File "/usr/local/lib/python3.7/site-packages/mopidy/commands.py", line 14, in <module>
    from mopidy.audio import Audio
  File "/usr/local/lib/python3.7/site-packages/mopidy/audio/__init__.py", line 2, in <module>
    from .actor import Audio
  File "/usr/local/lib/python3.7/site-packages/mopidy/audio/actor.py", line 8, in <module>
    from mopidy.audio import tags as tags_lib
  File "/usr/local/lib/python3.7/site-packages/mopidy/audio/tags.py", line 7, in <module>
    from mopidy.internal.gi import GLib, Gst
  File "/usr/local/lib/python3.7/site-packages/mopidy/internal/gi.py", line 5, in <module>
    import gi
ModuleNotFoundError: No module named 'gi'
kingosticks commented 4 years ago

Yes it should.

Since there's no .deb out yet, the idea is to follow https://docs.mopidy.com/en/latest/installation/pypi/.

However, if you are on Ubuntu 18.04, or something else that only ships with Python 3.6 as default, then it's a little more complicated; each Ubuntu release has a python3-gst-1.0 which is compiled with the system Python only. In which case you need to acquire a version of python3-gst-1.0 designed to work with Python 3.7 (or 3.8 if that's your target Python). For my Ubuntu 18.04 machine I ended up grabbing the cosmic version from https://packages.ubuntu.com/cosmic/python3-gst-1.0 but that's very hacky. We need better instructions for people in this situation @jodal .

But if you are working with docker you could just use our image?

fmarzocca commented 4 years ago

I suppose I have to wait for Iris migration to pyhton3 before installing mopidy 3, correct?

kingosticks commented 4 years ago

Yes.

On Wed, 25 Dec 2019, 11:02 Fabio Marzocca, notifications@github.com wrote:

I suppose I have to wait for Iris migration to pyhton3 before installing mopidy 3, correct?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jaedb/Iris/issues/356?email_source=notifications&email_token=AAHEHKDQ2D7A7KEHAVCLF6DQ2M4UBA5CNFSM4GTT2UNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHUIGMA#issuecomment-568886064, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHEHKBXWTLQWPAV6VTIPCLQ2M4UBANCNFSM4GTT2UNA .

MarcinWieczorek commented 4 years ago

Hello.

My current notes for Iris on mopidy 3 are:

This is useful

https://mopidy.com/blog/2019/12/27/mopidy-3-faq/

ModuleNotFoundError: No module named 'gi'

is solved easily in above page with pip install --ignore-installed --no-cache pygobject

Tornado is not compatible

I was still using outdated tornado to get Iris working and the issue is still valid. https://github.com/jupyter/notebook/issues/3397#issuecomment-377198047 hotfix: pip3 install tornado==4.5.3 (requires real fix)

Urlencode fix

Too little to create a pull request :)

From 9648c5cb8c0d5b20fc9f2de53ed66658566aaab6 Mon Sep 17 00:00:00 2001
From: Marcin Wieczorek <marcin@marcin.co>
Date: Mon, 30 Dec 2019 21:57:35 +0100
Subject: [PATCH] Fix urlencode

---
 mopidy_iris/core.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mopidy_iris/core.py b/mopidy_iris/core.py
index d9b21602..33b2227a 100755
--- a/mopidy_iris/core.py
+++ b/mopidy_iris/core.py
@@ -988,7 +988,7 @@ class IrisCore(pykka.ThreadingActor):

         try:
             http_client = tornado.httpclient.HTTPClient()
-            request = tornado.httpclient.HTTPRequest(url, method='POST', body=urllib.urlencode(data))
+            request = tornado.httpclient.HTTPRequest(url, method='POST', body=urllib.parse.urlencode(data))
             response = http_client.fetch(request)

             token = json.loads(response.body)
-- 
2.24.1

UI issue with notification:

Create process notification works, but cannot be canceled.

Run test process does not work, even blocking

I don't know what is the sudo for, but the fallback way doesn't seem to work.

Soooo after what I've wrote above it plays music!

jaedb commented 4 years ago

@MarcinWieczorek that's awesome, thanks heaps! Is that based off the current state of the feature/python3 branch?

MarcinWieczorek commented 4 years ago

Yes indeed

kingosticks commented 4 years ago

I finally got some time to try and help here as I promised last month. I see you've already done a load of the cookiecutter updates, which is great. So I did a bit more pathlib related stuff, and added some tests. You can see my progress at https://github.com/kingosticks/Iris/tree/feature/python3-py37 but it's a work in progress so no PR yet (I can do one if any of what I have got is useful to you right now, @jaedb).

brammittendorff commented 4 years ago

I'm waiting for this to, please let me know a list of what needs to be done. Maybe I can help on the feature/python3 branch. The project I am porting and waiting for this (Iris mopidy3) to be finished is: https://github.com/hassio-addons/addon-mopidy/pull/20/files.

jaedb commented 4 years ago

@kingosticks thanks for your contribution, it is looking great! I like what you've done to split the handlers out too.

I'll merge it in with my progress and see how it's all hanging together.

I have been a bit stuck on the system tasks (system.py) where threading is concerned. The issue seems to be in handling the callback and delivering the response via websocket:

Exception in thread Thread-10:
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/threading.py", line 926, in _bootstrap_inner
    self.run()
  File "/iris/mopidy_iris/system.py", line 48, in run
    self.callback({ 'output': stdout.decode() }, None)
  File "/iris/mopidy_iris/core.py", line 553, in local_scan_callback
    'params': response
  File "/iris/mopidy_iris/core.py", line 245, in broadcast
    connection['connection'].write_message(json_encode(message))
  File "/root/.local/lib/python3.7/site-packages/tornado/websocket.py", line 342, in write_message
    return self.ws_connection.write_message(message, binary=binary)
  File "/root/.local/lib/python3.7/site-packages/tornado/websocket.py", line 1098, in write_message
    fut = self._write_frame(True, opcode, message, flags=flags)
  File "/root/.local/lib/python3.7/site-packages/tornado/websocket.py", line 1075, in _write_frame
    return self.stream.write(frame)
  File "/root/.local/lib/python3.7/site-packages/tornado/iostream.py", line 555, in write
    future = Future()  # type: Future[None]
  File "/usr/local/lib/python3.7/asyncio/events.py", line 644, in get_event_loop
    % threading.current_thread().name)
RuntimeError: There is no current event loop in thread 'Thread-10'.

I'm really not sure I'm handling the single-instance IrisCore class properly (stored in mem.py).

MarcinWieczorek commented 4 years ago

@jaedb I don't remember details, but I've read somewhere about tornado's threading changes. Maybe upgrading it would make things easier. Look at my previous comment.

kingosticks commented 4 years ago

This looks to be the same thing as I mentioned in #275. Those broadcasts need to run on the eventloop running in the server thread.

jaedb commented 4 years ago

Keen for your thoughts on https://github.com/jaedb/Iris/commit/564839c92b0e43a058bfbb58f4c6d39c2abf566c. Essentially Frontend uses IrisCore's ioloop that is provided by the Iris websocket in handlers.py.

fmarzocca commented 4 years ago

I have seen there is a new release. Is this issue fixed?

fmauNeko commented 4 years ago

The changelog says

Support for Mopidy 3 / python 3.8 No backwards compatibility to Mopidy 2 / python 2.7

So I guess that must indeed be solved. The package isn't released on PyPI yet though.

fmarzocca commented 4 years ago

The package isn't released on PyPI yet though.

thanks. How can I get informed about pip3 release of Iris? Will it be also added to mopidy's apt repository?

tgurr commented 4 years ago

I'd also welcome a release on PyPI so we can update our distribution package to the latest Iris version.

@jaedb One thing I noticed in the tarball from Github is that setup.cfg has version = 3.0.0 shouldn't it state 3.44.0 instead? And there's also a file VERSION included which has 3.7.5-2 in it.

Gadgetoid commented 4 years ago

Thank you for getting this updated so quickly @jaedb - I can't imagine a Mopidy without the Iris front-end. Eagerly awaiting a pip release!

MarcinWieczorek commented 4 years ago

I'm glad to see the update. @jaedb, has anything changed about the dependendies? I'm about to update the package in AUR.

sandyjmacdonald commented 4 years ago

Nice work, @jaedb! It's working well for me. Also eagerly awaiting a pip release :-)

JayGatsby7 commented 4 years ago

Nice work, @jaedb! It's working well for me. Also eagerly awaiting a pip release :-)

Wouldn’t mind seeing some idiot proof instructions for how to install this on a raspberry pi using docker. I have zero experience with docker but like the idea of it from a security standpoint.

MarcinWieczorek commented 4 years ago

Nice work, @jaedb! It's working well for me. Also eagerly awaiting a pip release :-)

Wouldn’t mind seeing some idiot proof instructions for how to install this on a raspberry pi using docker. I have zero experience with docker but like the idea of it from a security standpoint.

pip install mopidy-whatever in dockerfile https://github.com/wernight/docker-mopidy

JayGatsby7 commented 4 years ago

Ahh so the docker container is mopidy. Now I see

JayGatsby7 commented 4 years ago

That doesn’t seem practical on a raspberry pi

MarcinWieczorek commented 4 years ago

That doesn’t seem practical on a raspberry pi

It's as practical as it gets. You can have mopidy separated from python packages which is good (I think I'm going to switch to that soon). It's similar to just using a virtualenv, but with docker.

jaedb commented 4 years ago

Iris does have a Docker container and will be uploaded to Docker Hub when the final stages of release are finished.

jaedb commented 4 years ago

@kingosticks do you have any advice on the check_manifest tox test? The /src path doesn't need to be in the distribution (but it does need to be in VCS) and I can't see how it can be omitted without breaking the test.

BrainStone commented 4 years ago

Is there a somewhat working test version I could try at the moment?

jodal commented 4 years ago

@kingosticks do you have any advice on the check_manifest tox test? The /src path doesn't need to be in the distribution (but it does need to be in VCS) and I can't see how it can be omitted without breaking the test.

You can ignore src/ in a check-manifest section in pyproject.toml, ref. https://github.com/mgedmin/check-manifest#configuration

kingosticks commented 4 years ago

But isn't that already specified at https://github.com/jaedb/Iris/blob/master/tox.ini#L28 ?

Is this because it should be src/** in there (are they even globs?). EDIT: they are not globs, it uses fnmatch.

jaedb commented 4 years ago

@BrainStone yes, you should be able install from source using the develop branch:

  1. Clone this repo
  2. Checkout develop branch
  3. Run python3 -m setup.py install
BrainStone commented 4 years ago

@jaedb would something like python3 -m pip install https://github.com/jaedb/Iris/archive/develop.zip work too?

Edit: Yes it does.

brammittendorff commented 4 years ago

When using the following packages:

Mopidy==3.0.1
Pykka==2.0.2
Mopidy-Local==3.1.1
Mopidy-AlsaMixer==2.0.0
Mopidy-Spotify==4.0.1
git+https://github.com/jaedb/Iris.git@develop
Mopidy-SoundCloud==3.0.0
Mopidy-GMusic==4.0.0
Mopidy-Youtube==3.0a1

Working:

Not working

After searching with Spotify i get this error:

ERROR    2020-02-07 13:29:52,322 [418:HttpServer] tornado.application
  Uncaught exception
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/dist-packages/tornado/http1connection.py", line 238, in _read_message
    delegate.finish()
  File "/usr/local/lib/python3.7/dist-packages/tornado/routing.py", line 251, in finish
    self.delegate.finish()
  File "/usr/local/lib/python3.7/dist-packages/tornado/web.py", line 2097, in finish
    self.execute()
  File "/usr/local/lib/python3.7/dist-packages/tornado/web.py", line 2117, in execute
    **self.handler_kwargs)
2020/02/07 13:29:52 [error] 542#542: *15 upstream prematurely closed connection while reading response header from upstream, client: 172.30.32.2, server: local-mopidy, request: "GET /iris/http/refresh_spotify_token HTTP/1.1", upstream: "http://127.0.0.1:4478/iris/http/refresh_spotify_token", host: "fred.local", referrer: "http://fred.local/iris/search/all/test"
  File "/usr/local/lib/python3.7/dist-packages/tornado/web.py", line 190, in __init__
    self.clear()
  File "/usr/local/lib/python3.7/dist-packages/tornado/web.py", line 294, in clear
    self.set_default_headers()
  File "/usr/local/lib/python3.7/dist-packages/mopidy_iris/handlers.py", line 183, in set_default_headers
    Authorization, Client-Security-Token, Accept-Encoding''',
  File "/usr/local/lib/python3.7/dist-packages/tornado/web.py", line 339, in set_header
[07/Feb/2020:13:29:52 +0100] 502 10.53.8.94, 172.30.33.6, 172.30.32.1(172.30.32.2) GET /iris/http/refresh_spotify_token HTTP/1.1 (Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/79.0.3945.79 Chrome/79.0.3945.79 Safari/537.36)
    self._headers[name] = self._convert_header_value(value)
  File "/usr/local/lib/python3.7/dist-packages/tornado/web.py", line 388, in _convert_header_value
    raise ValueError("Unsafe header value %r", retval)
ValueError: ('Unsafe header value %r', 'Origin, X-Requested-With, Content-Type, Accept,\n            Authorization, Client-Security-Token, Accept-Encoding')

Even with tornado-4.5.3:

Successfully built tornado
Installing collected packages: tornado
  Attempting uninstall: tornado
    Found existing installation: tornado 6.0.3
    Uninstalling tornado-6.0.3:
      Successfully uninstalled tornado-6.0.3
Successfully installed tornado-4.5.3
jjok commented 4 years ago

@brammittendorff I had that problem too, but then I installed https://github.com/jaedb/Iris/archive/3.44.0.zip instead of develop.zip and it worked.

brammittendorff commented 4 years ago

@jjok You are right that one works, even with Spotify thanks alot. So the working setup is:

Mopidy==3.0.1
Pykka==2.0.2
Mopidy-Local==3.1.1
Mopidy-AlsaMixer==2.0.0
Mopidy-Spotify==4.0.1
git+https://github.com/jaedb/Iris.git@3.44.0
Mopidy-SoundCloud==3.0.0
Mopidy-GMusic==4.0.0
Mopidy-Youtube==3.0a1
jaedb commented 4 years ago

This has just been released to pip and the image to Docker hub so I shall close this issue. Please open a new issue for any problems you may encounter.

fmarzocca commented 4 years ago

I have found only v.3.43.0 in pip3.

JayGatsby7 commented 4 years ago

I have found only v.3.43.0 in pip3.

sudo python3 -m pip install Mopidy-Iris [should work for you]