Python 3 support #75

pbauer · 2018-06-22T16:27:48Z

No description provided.

coveralls · 2018-06-22T17:22:28Z

Coverage decreased (-0.02%) to 97.731% when pulling 8f5c6c9 on python3 into 0078eda on master.

davisagli · 2018-06-22T19:37:25Z

src/plone/rest/testing.py

@@ -41,7 +41,7 @@ def setUpZope(self, app, configurationContext):
 class InternalServerErrorService(Service):

    def __call__(self):
-        from urllib2 import HTTPError
+        from six.moves.urllib.error import HTTPError


We should declare the dependency on six in setup.py

pbauer · 2018-06-28T09:24:41Z

Tests in py3 are: 96 tests, 8 failures, 0 errors, 0 skipped.

hvelarde · 2018-07-13T22:11:08Z

src/plone/rest/tests/test_permissions.py

@@ -41,8 +41,9 @@ def traverse(self, path='/plone', accept='application/json', method='GET'):
        request.environ['PATH_TRANSLATED'] = path
        request.environ['HTTP_ACCEPT'] = accept
        request.environ['REQUEST_METHOD'] = method
-        request._auth = 'Basic %s' % b64encode(
-            '%s:%s' % (TEST_USER_NAME, TEST_USER_PASSWORD))
+        auth = '%s:%s' % (TEST_USER_NAME, TEST_USER_PASSWORD)


shouldn't we use .format() instead?

hvelarde · 2018-07-13T22:11:26Z

src/plone/rest/tests/test_permissions.py

-            '%s:%s' % (TEST_USER_NAME, TEST_USER_PASSWORD))
+        auth = '%s:%s' % (TEST_USER_NAME, TEST_USER_PASSWORD)
+        b64auth = b64encode(auth.encode('utf8'))
+        request._auth = 'Basic %s' % b64auth.decode('utf8')


shouldn't we use .format() instead?

hvelarde · 2018-07-13T22:11:35Z

src/plone/rest/tests/test_traversal.py

@@ -32,8 +32,9 @@ def traverse(self, path='/plone', accept='application/json', method='GET'):
        request.environ['PATH_TRANSLATED'] = path
        request.environ['HTTP_ACCEPT'] = accept
        request.environ['REQUEST_METHOD'] = method
-        request._auth = 'Basic %s' % b64encode(
-            '%s:%s' % (SITE_OWNER_NAME, SITE_OWNER_PASSWORD))
+        auth = '%s:%s' % (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)


shouldn't we use .format() instead?

hvelarde · 2018-07-13T22:11:41Z

src/plone/rest/tests/test_traversal.py

-            '%s:%s' % (SITE_OWNER_NAME, SITE_OWNER_PASSWORD))
+        auth = '%s:%s' % (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)
+        b64auth = b64encode(auth.encode('utf8'))
+        request._auth = 'Basic %s' % b64auth.decode('utf8')


shouldn't we use .format() instead?

hvelarde · 2018-07-13T22:12:50Z

this seems to be almost ready now:

$ pylint --py3k --disable=no-absolute-import src/plone/rest/
No config file found, using default configuration

------------------------------------
Your code has been rated at 10.00/10

lukasgraf · 2018-07-13T22:42:06Z

Regarding the remaining 8 failures on Plone 5.2 / py3.cfg:

These aren't Python 3 failures as such, but come from the difference between ZServer.ZPublisher and ZPublisher.WSGIPublisher.

7 of the 8 failures are because status 401 doesn't get returned where it's expected:

AssertionError: The following assertions failed:

Request:  ('/', 'POST', ())
Expected: 401
Got:      404
...

This is because in WSGIPublisher, the response._unauthorized() gets triggered before the exception view is called. In ZPublisher this happens the other way round, and plone.rest gets a change to prevent PAS from issuing a challenge and triggering a redirect to the login form.

The remaining failure is caused by WSGIPublisher normalizing HTTP exceptions. This leads to the last actual exception (sys.exc_info()) being different from the exception that the error view gets, and causing plone.rest to fail on this check:

AssertionError: 'ERROR: Another exception happened before we could render the traceback.' is not an instance of <class 'list'>

pbauer · 2018-07-16T16:16:40Z

@davisagli or @tschorr could one of you have a look at this? I'm notoriously bad when it comes to understanding the various layers of exception-handling.

tschorr · 2018-07-19T08:30:51Z

The remaining failure is caused by WSGIPublisher normalizing HTTP exceptions.

Always when I see x-wsgiorg.throw_errors in code (as in the 2 lines above the Unathorized handling), I wonder wether this is used anywhere outside of Zope. The specification for this was created in 2006 and is still in proposed state ...

Maybe get rid of x-wsgiorg.throw_errors altogether?

tschorr · 2018-07-19T10:21:08Z

Running bin/test -s plone.rest with zopefoundation/Zope#294 yields 3 failures and 1 error:

Failure in test test_401_unauthorized (plone.rest.tests.test_error_handling.TestErrorHandling)
Failure in test test_404_not_found (plone.rest.tests.test_error_handling.TestErrorHandling)
Failure in test test_500_internal_server_error (plone.rest.tests.test_error_handling.TestErrorHandling)

all fail with:

AssertionError: 'text/html;charset=utf-8' != 'application/json'
- text/html;charset=utf-8
+ application/json
 : When sending a GET request with Accept: application/json the server should respond with sending back application/json.

and then there's

Error in test test_500_traceback_only_for_manager_users (plone.rest.tests.test_error_handling.TestErrorHandling)
Traceback (most recent call last):
  File "/home/thomas/.pyenv/versions/3.6.5/lib/python3.6/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/home/thomas/.pyenv/versions/3.6.5/lib/python3.6/unittest/case.py", line 605, in run
    testMethod()
  File "/home/thomas/devel/plone/buildout.coredev/src/plone.rest/src/plone/rest/tests/test_error_handling.py", line 107, in test_500_traceback_only_for_manager_users
    self.assertNotIn(u'traceback', response.json())
  File "/home/thomas/.buildout/eggs/cp36m/requests-2.19.1-py3.6.egg/requests/models.py", line 896, in json
    return complexjson.loads(self.text, **kwargs)
  File "/home/thomas/.buildout/eggs/cp36m/simplejson-3.16.0-py3.6-linux-x86_64.egg/simplejson/__init__.py", line 518, in loads
    return _default_decoder.decode(s)
  File "/home/thomas/.buildout/eggs/cp36m/simplejson-3.16.0-py3.6-linux-x86_64.egg/simplejson/decoder.py", line 370, in decode
    obj, end = self.raw_decode(s)
  File "/home/thomas/.buildout/eggs/cp36m/simplejson-3.16.0-py3.6-linux-x86_64.egg/simplejson/decoder.py", line 400, in raw_decode
    return self.scan_once(s, idx=_w(s, idx).end())
simplejson.errors.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

davisagli · 2018-07-19T12:45:39Z

@tschorr that's because you disabled exception view rendering, which is how plone.rest turns exceptions into JSON responses.

I would need to take a closer look at how to fix the problems that @lukasgraf pointed out, but getting rid of exception view rendering is not it.

…ukasgraf in plone/plone.rest#75 (comment)

tschorr · 2018-07-20T09:09:30Z

This is because in WSGIPublisher, the response._unauthorized() gets triggered before the exception view is called. In ZPublisher this happens the other way round, and plone.rest gets a change to prevent PAS from issuing a challenge and triggering a redirect to the login form.

@lukasgraf I have reversed the order according to your proposal in https://github.com/zopefoundation/Zope/tree/py3_fix_wsgi_unauthorized and confirm this fixes 7 tests. Tests in Zope all pass with this modification.

tschorr · 2018-07-20T09:13:58Z

Jenkins looks slightly better using py3_fix_wsgi_unauthorized: https://jenkins.plone.org/view/PLIPs/job/plip-py3/582

tschorr · 2018-07-20T10:33:11Z

Checks for this PR pass now maybe because only Python 2.7 is tested with Travis. I still have the one failing test mentioned by @lukasgraf ("Another exception ...") using Python 3.6.

tisto · 2018-07-20T10:52:57Z

@tschorr feel free to add Python 3 to the Travis configuration.

lukasgraf · 2018-07-20T12:00:18Z

@tschorr thanks for looking into this!

Just to clarify, I'm not necessarily proposing that the order in WSGIPublisher needs to be changed - I just noticed that this is the underlying cause of these failures in plone.rest. I left some feedback in my comment on #304.

…ukasgraf in plone/plone.rest#75 (comment)

This reverts commit 587b3a6.

src/plone/rest/errors.py

…aw response

frapell · 2018-10-04T15:04:51Z

@pbauer I see this is hitting plone/plone.protect#74 even though it is using python 2.7.14 (see latest travis https://travis-ci.org/plone/plone.rest/jobs/436770003)

Do we need a new release of plone.protect with the latest changes for tests to pass?

tisto · 2018-11-08T05:48:17Z

@jensens FYI: you merged this PR even though the Travis build failed. Please keep in mind that plone.rest and plone.restapi are used outside of Plone and for older Plone versions. It is just not sufficient if coredev tests pass.

pbauer added the 22 status: in-progress label Jun 22, 2018

davisagli reviewed Jun 22, 2018

View reviewed changes

use decorators

d2c75eb

pbauer force-pushed the python3 branch from 81af979 to d2c75eb Compare June 23, 2018 12:54

pbauer added 2 commits June 23, 2018 14:55

fix render_exceptions for py3

a549adc

start porting tests to py3

bbfa49d

pbauer mentioned this pull request Jul 13, 2018

Port plone.rest and plone.restapi to Python 3 plone/Products.CMFPlone#2474

Closed

hvelarde reviewed Jul 13, 2018

View reviewed changes

This was referenced Jul 14, 2018

PLIP: Port Plone to Python 3 plone/Products.CMFPlone#2368

Closed

[WIP] Python 3 support plone/plone.restapi#542

Merged

tschorr mentioned this pull request Jul 19, 2018

WIP: get rid of x-wsgi.throw errors zopefoundation/Zope#294

Closed

tschorr added a commit to zopefoundation/Zope that referenced this pull request Jul 20, 2018

call exception view before triggering _unaothorized, as proposed by l…

5263ce0

…ukasgraf in plone/plone.rest#75 (comment)

tschorr mentioned this pull request Jul 20, 2018

create exception view before handling Unauthorized zopefoundation/Zope#304

Merged

tschorr added 2 commits July 20, 2018 13:01

add python 3.6

ab46308

exclude python3.6, Plone < 5.1

8f5c6c9

pbauer pushed a commit to zopefoundation/Zope that referenced this pull request Sep 14, 2018

call exception view before triggering _unaothorized, as proposed by l…

a98491a

…ukasgraf in plone/plone.rest#75 (comment)

davisagli pushed a commit to zopefoundation/Zope that referenced this pull request Oct 2, 2018

call exception view before triggering _unaothorized, as proposed by l…

fe246b9

…ukasgraf in plone/plone.rest#75 (comment)

frapell added 11 commits October 3, 2018 02:44

Update classifiers

da1ce26

Update CHANGES.rst

e7d0a90

If request is a WSGIRequest, do not check the exception when rendering

96599cc

Fix test

7788ba2

Merge branch 'master' into python3

6861367

Do not print error if on Python 3

3376cc8

Bad merge

3636448

Fix tests

396d16a

Fix deprecation warnings and good practices

587b3a6

Do not return an error when exceptions differ, if on a WSGIRequest

9f89870

Revert "Fix deprecation warnings and good practices"

28aeeeb

This reverts commit 587b3a6.

hvelarde reviewed Oct 3, 2018

View reviewed changes

src/plone/rest/errors.py Outdated Show resolved Hide resolved

frapell added 5 commits October 3, 2018 16:20

Good practices when handling files

ebaa24f

Specify exception

6e0f7af

response.text is 'null' in py3 and u'' in py2, so compare using the r…

4a81582

…aw response

flake8

684de9e

Use plone.protect 4.0.1 for travis

e20c000

jensens merged commit c30de1e into master Oct 17, 2018

jensens deleted the python3 branch October 17, 2018 09:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python 3 support #75

Python 3 support #75

pbauer commented Jun 22, 2018

coveralls commented Jun 22, 2018 •

edited

Loading

davisagli Jun 22, 2018

pbauer commented Jun 28, 2018

hvelarde Jul 13, 2018

hvelarde Jul 13, 2018

hvelarde Jul 13, 2018

hvelarde Jul 13, 2018

hvelarde commented Jul 13, 2018

lukasgraf commented Jul 13, 2018 •

edited

Loading

pbauer commented Jul 16, 2018

tschorr commented Jul 19, 2018 •

edited

Loading

tschorr commented Jul 19, 2018

davisagli commented Jul 19, 2018 •

edited

Loading

tschorr commented Jul 20, 2018

tschorr commented Jul 20, 2018

tschorr commented Jul 20, 2018 •

edited

Loading

tisto commented Jul 20, 2018

lukasgraf commented Jul 20, 2018

frapell commented Oct 4, 2018

tisto commented Nov 8, 2018

Python 3 support #75

Python 3 support #75

Conversation

pbauer commented Jun 22, 2018

coveralls commented Jun 22, 2018 • edited Loading

davisagli Jun 22, 2018

Choose a reason for hiding this comment

pbauer commented Jun 28, 2018

hvelarde Jul 13, 2018

Choose a reason for hiding this comment

hvelarde Jul 13, 2018

Choose a reason for hiding this comment

hvelarde Jul 13, 2018

Choose a reason for hiding this comment

hvelarde Jul 13, 2018

Choose a reason for hiding this comment

hvelarde commented Jul 13, 2018

lukasgraf commented Jul 13, 2018 • edited Loading

pbauer commented Jul 16, 2018

tschorr commented Jul 19, 2018 • edited Loading

tschorr commented Jul 19, 2018

davisagli commented Jul 19, 2018 • edited Loading

tschorr commented Jul 20, 2018

tschorr commented Jul 20, 2018

tschorr commented Jul 20, 2018 • edited Loading

tisto commented Jul 20, 2018

lukasgraf commented Jul 20, 2018

frapell commented Oct 4, 2018

tisto commented Nov 8, 2018

coveralls commented Jun 22, 2018 •

edited

Loading

lukasgraf commented Jul 13, 2018 •

edited

Loading

tschorr commented Jul 19, 2018 •

edited

Loading

davisagli commented Jul 19, 2018 •

edited

Loading

tschorr commented Jul 20, 2018 •

edited

Loading