Fixes for Python 3 migration #211

CastagnaIT · 2019-09-13T22:34:01Z

Check if this PR fulfills these requirements:

My changes respect the Kodi add-on development rules
I have read the CONTRIBUTING document.
I made sure there wasn't another one Pull Requests opened for the same update/change
I have successfully tested my changes locally

Types of changes

New feature (non-breaking change which adds functionality)
Feature change (non-breaking change which change behaviour of an existing functionality)
Improvement (non-breaking change which improve functionality)
Bug fix (non-breaking change which fixes an issue)
Breaking change (fix or feature that would cause existing functionality to change)

Description

Porting of the PR #198
Fixes for migration to python 3, this include also fixes to allow execution with the both versions of Python:
Kodi 18.x >> Python 2
Kodi 19.x >> Python 3

Tests:
All tests are performed with a spanish profile in order to test unicode special chars cases
in follow devices:
Kodi 19.x on LibreELEC (at now the only build ready with Py3 and reuselanguageinvoker)
Kodi 18.x on Windows

List of tests:
✔Addon start
✔Cache
✔Cookies
✔Loading lists
✔Play videos
✔Library (export/remove/update context menu)
✔Auto-update all settings features
✔NFO files
✔Shared library MySQL
✔In general test all settings options
✔UpNext integration
...Other?

Who wants to give a concrete help means to verify continuously the output of the logs for every operation carried out so not only what you see on screen

In case of Feature change / Breaking change:

Describe the current behavior

Describe the new behavior

Screenshots (if appropriate):

CastagnaIT · 2019-09-13T22:43:16Z

I had to make the corrections you didn't make

CastagnaIT · 2019-09-13T22:43:59Z

commits are temporary

CastagnaIT · 2019-09-13T22:44:55Z

you said you didn't want responsibility to break the addon so i thought I'd do it this way...

CastagnaIT · 2019-09-13T22:57:46Z

so wait, I'll try to join

dagwieers · 2019-09-13T23:03:43Z

You know what, I'll close my PR. Since you already have been adding more commits to this PR it is probably going to be a lot easier.

CastagnaIT · 2019-09-13T23:06:56Z

I was doing the pull on your branch ... that's why it went in error... ok as you prefer

CastagnaIT · 2019-09-13T23:10:26Z

how do run integrity tests under windows?

dagwieers · 2019-09-14T00:08:49Z

Look in the Makefile for the right commands. In Linux you can just do make test or make run.
But on Windows you have to run the actual commands manually.

dagwieers · 2019-09-14T00:25:31Z

BTW I still need to add routing integration tests, but I added the test/run.py so you can start the service in the background and then run the plugin with different routes to see how it behaves.

CastagnaIT · 2019-09-14T12:34:21Z

I'd like to make a release, do you think it's better to wait and include this PR or can i proceed?

Smeulf · 2019-09-14T12:47:47Z

I have to check for MySQL again with some specific use cases if you don't mind, but I think I can do it before the evening.

dagwieers · 2019-09-14T12:48:29Z

As long as everything has been tested on Python 2.7 I think we're good. I haven't seen any issues (run-time or visual defects) on Python 2.7 with my PR during end-to-end testing. But we currently do not test all code-paths as we should so there's no real guarantee. (And there wasn't a guarantee before either ;-))

CastagnaIT · 2019-09-14T12:53:51Z

I only did a quick test with mysql...a more thorough one is appreciated
so well at least we're going the right way 😊 then to now i put it as WIP

dagwieers · 2019-09-14T13:02:27Z

@CastagnaIT I wasn't talking about MySQL specifically, there is a lot more code-paths we don't cover (see coverage results). And we don't have proper tests for most of the input/output functionality in Kodi/python to know for sure that we handle everything correctly wrt. unicode.

I am struggling with this for other add-ons as well, but it shouldn't be worse than before we added tests ;-)

Update: We don't have coverage reports at the moment because we do not run any of these tests yet. (See .travis) https://codecov.io/gh/dagwieers/plugin.video.netflix/branch/preparation

Smeulf · 2019-09-14T22:06:31Z

resources/lib/common/kodiops.py

@@ -37,7 +40,7 @@ def json_rpc(method, params=None):
                    'params': params or {}}
    request = json.dumps(request_data)
    debug('Executing JSON-RPC: {}'.format(request))
-    raw_response = unicode(xbmc.executeJSONRPC(request), 'utf-8')
+    raw_response = text(xbmc.executeJSONRPC(request)).encode('utf-8')


With the series "3%" exported, I can't start an episode. I always get a UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 1013: ordinal not in range(128). This is not related to any accentuated chars, it does the same with series with no accent or anything special in the name of the series or the episode.

I don't know how to fix it, and I fear all text() functions are suffering the issue :/

See here for the first error in json_rpc() https://paste.kodi.tv/vitenogebu.kodi
And here for the same kind of error in _decrypt_chunks() https://paste.kodi.tv/olinafikon.kodi

Till this error exists, I can't go further. I'm sorry I can't help more.

@Smeulf You have an actual Kodi with Python 3 support that works?

No it's Python 2 (Kodi 18.2, "prod" Raspberry Pi)

But that's not the exact same code you proposed in your PR, yours was raw_response = text(xbmc.executeJSONRPC(request), 'utf-8')

yes is not same becouse the text(xbmc.executeJSONRPC(request), 'utf-8') do not work with py2, str do not have parameters...

I thought it worked, i hate UnicodeDecodeError is a ugly beast

do not work with py2, str do not have parameters...

'str' or 'text' ? @dagwieers was using 'text'

This works fine on Python 2:

from builtins import str as text print("%r" % text(u"wieërs", "utf-8"))

Returns 'wie\xc3\xabrs'

text is only a reference to str (in py2) look to the imports

dagwieers · 2019-09-16T18:30:01Z

Using text() was just a hack, I think we need to move to dedicated functions. Elsewhere we use this:

def to_unicode(text, encoding='utf-8'):
    ''' Force text to unicode '''
    return text.decode(encoding) if isinstance(text, bytes) else text


def from_unicode(text, encoding='utf-8'):
    ''' Force unicode to text '''
    import sys
    if sys.version_info.major == 2 and isinstance(text, unicode):  # noqa: F821; pylint: disable=undefined-variable
        return text.encode(encoding)
    return text

I was planning to implement this, before my branch was hijacked ;-)

CastagnaIT · 2019-09-16T18:41:35Z

I want to try again but tonight i can't

CastagnaIT · 2019-09-17T19:27:30Z

now it should work,
"unicode" is used by py2
"str" in py3 is same unicode

CastagnaIT · 2019-09-21T11:28:10Z

I have installed WSL Ubuntu with python and moudule requirements
with make unit/test i have only this output, it seems strange...

This fixes the known Python 3 issues reported by kodi-addon-checker. Also includes various known fixes for Python 3 Co-authored-by: Stefano Gottardo <[email protected]>

#183

…ll inspection

now the installation is done correctly

CastagnaIT · 2019-11-10T17:32:46Z

I plan the merge to master in one/two days
if anyone has to communicate something do it now
any improvements on the code if necessary will be made on other PRs

notoco · 2019-11-12T09:12:24Z

Something does not work this version - at first a problem with Polish characters (ś), then addonsignals timeout
https://hastebin.com/abanamoyig.sql

The stable version obviously works.
Checked both on the old netflix profile in kodi, as well as on a completely new one

CastagnaIT · 2019-11-12T09:29:59Z

Can you try this?
plugin.video.netflix_0.15.8_20191112.zip

notoco · 2019-11-12T09:39:22Z

This version works

This comment has been minimized.

Sign in to view

CastagnaIT force-pushed the python3 branch from 56c333a to 98a763c Compare September 13, 2019 22:40

This comment has been minimized.

Sign in to view

CastagnaIT force-pushed the python3 branch from 98a763c to 6b0a505 Compare September 13, 2019 23:19

CastagnaIT mentioned this pull request Sep 14, 2019

Get rid of Enum #204

Closed

CastagnaIT force-pushed the python3 branch from 6b0a505 to 1055cea Compare September 14, 2019 12:37

CastagnaIT added the WIP PR that is still being worked on label Sep 14, 2019

Smeulf reviewed Sep 14, 2019

View reviewed changes

CastagnaIT force-pushed the python3 branch 2 times, most recently from ceb5848 to 9d063d1 Compare September 17, 2019 19:23

CastagnaIT force-pushed the python3 branch from 9d063d1 to 61d882a Compare September 21, 2019 11:23

CastagnaIT and others added 21 commits November 9, 2019 18:48

Deleted enum module

874b2ea

Fix known Python 3 issues

2abfa48

This fixes the known Python 3 issues reported by kodi-addon-checker. Also includes various known fixes for Python 3 Co-authored-by: Stefano Gottardo <[email protected]>

uuid_device py3 fixes

7981176

xmldialogs.py: Remove unused class

6c2dc0c

#183

Drop # noinspection instructions for PyCharm, we now use Pylint for a…

727449d

…ll inspection

Unicode fixes

1c012f5

Fixed pylint unnecessary-comprehension

01bc694

Disabled pylint import-outside-toplevel

0719a54

Traceback fixes

4a5f088

Exception message fix pylint happy

04a621e

Use future version 0.17.1

f165bae

now the installation is done correctly

Pylint fixes

f7ea137

Exception message fix

e3132af

Show traceback when an error occour in service

ac193f8

Rebase py3 fixes

e7bbcd3

Fixes for py3 and py2

f809849

Avoid generate exception due to cookie file mmissing

74e40f8

Fixed pickle.loads on _get_from_disk

f8120b2

Workaround to py2 bug ImportError: Failed to import _strptime

5afa451

Raise CacheMiss directly when file does not exist

46bc32c

flake - pylint fixes

dbb832e

CastagnaIT force-pushed the python3 branch from 0945590 to dbb832e Compare November 9, 2019 17:48

CastagnaIT removed the WIP PR that is still being worked on label Nov 10, 2019

ccope mentioned this pull request Nov 11, 2019

Update to python3 support #293

Closed

CastagnaIT merged commit 3e6ca65 into master Nov 11, 2019

CastagnaIT deleted the python3 branch November 11, 2019 17:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes for Python 3 migration #211

Fixes for Python 3 migration #211

CastagnaIT commented Sep 13, 2019 •

edited

Loading

This comment has been minimized.

CastagnaIT commented Sep 13, 2019

CastagnaIT commented Sep 13, 2019

CastagnaIT commented Sep 13, 2019

This comment has been minimized.

CastagnaIT commented Sep 13, 2019

This comment has been minimized.

dagwieers commented Sep 13, 2019

CastagnaIT commented Sep 13, 2019

CastagnaIT commented Sep 13, 2019

dagwieers commented Sep 14, 2019 •

edited

Loading

dagwieers commented Sep 14, 2019

CastagnaIT commented Sep 14, 2019

Smeulf commented Sep 14, 2019

dagwieers commented Sep 14, 2019

CastagnaIT commented Sep 14, 2019

dagwieers commented Sep 14, 2019 •

edited

Loading

Smeulf Sep 14, 2019

dagwieers Sep 14, 2019

Smeulf Sep 14, 2019

Smeulf Sep 14, 2019 •

edited

Loading

CastagnaIT Sep 16, 2019 •

edited

Loading

CastagnaIT Sep 16, 2019

Smeulf Sep 16, 2019

dagwieers Sep 16, 2019

CastagnaIT Sep 16, 2019

dagwieers commented Sep 16, 2019

CastagnaIT commented Sep 16, 2019

CastagnaIT commented Sep 17, 2019

CastagnaIT commented Sep 21, 2019

CastagnaIT commented Nov 10, 2019

notoco commented Nov 12, 2019 •

edited

Loading

CastagnaIT commented Nov 12, 2019

notoco commented Nov 12, 2019

Fixes for Python 3 migration #211

Fixes for Python 3 migration #211

Conversation

CastagnaIT commented Sep 13, 2019 • edited Loading

Check if this PR fulfills these requirements:

Types of changes

Description

In case of Feature change / Breaking change:

Describe the current behavior

Describe the new behavior

Screenshots (if appropriate):

This comment has been minimized.

CastagnaIT commented Sep 13, 2019

CastagnaIT commented Sep 13, 2019

CastagnaIT commented Sep 13, 2019

This comment has been minimized.

CastagnaIT commented Sep 13, 2019

This comment has been minimized.

dagwieers commented Sep 13, 2019

CastagnaIT commented Sep 13, 2019

CastagnaIT commented Sep 13, 2019

dagwieers commented Sep 14, 2019 • edited Loading

dagwieers commented Sep 14, 2019

CastagnaIT commented Sep 14, 2019

Smeulf commented Sep 14, 2019

dagwieers commented Sep 14, 2019

CastagnaIT commented Sep 14, 2019

dagwieers commented Sep 14, 2019 • edited Loading

Smeulf Sep 14, 2019

Choose a reason for hiding this comment

dagwieers Sep 14, 2019

Choose a reason for hiding this comment

Smeulf Sep 14, 2019

Choose a reason for hiding this comment

Smeulf Sep 14, 2019 • edited Loading

Choose a reason for hiding this comment

CastagnaIT Sep 16, 2019 • edited Loading

Choose a reason for hiding this comment

CastagnaIT Sep 16, 2019

Choose a reason for hiding this comment

Smeulf Sep 16, 2019

Choose a reason for hiding this comment

dagwieers Sep 16, 2019

Choose a reason for hiding this comment

CastagnaIT Sep 16, 2019

Choose a reason for hiding this comment

dagwieers commented Sep 16, 2019

CastagnaIT commented Sep 16, 2019

CastagnaIT commented Sep 17, 2019

CastagnaIT commented Sep 21, 2019

CastagnaIT commented Nov 10, 2019

notoco commented Nov 12, 2019 • edited Loading

CastagnaIT commented Nov 12, 2019

notoco commented Nov 12, 2019

CastagnaIT commented Sep 13, 2019 •

edited

Loading

dagwieers commented Sep 14, 2019 •

edited

Loading

dagwieers commented Sep 14, 2019 •

edited

Loading

Smeulf Sep 14, 2019 •

edited

Loading

CastagnaIT Sep 16, 2019 •

edited

Loading

notoco commented Nov 12, 2019 •

edited

Loading