zopefoundation / zope.publisher

Map requests from HTTP/WebDAV clients, web browsers, XML-RPC and FTP clients onto Python objects
Other
3 stars 13 forks source link

Compatibility with PythonScripts #10

Closed thet closed 7 years ago

thet commented 8 years ago

Use six to access the function object and function code in zope.publisher.publisher.unwrapMethod. This restores compatibility with Products.PythonScripts, where parameters were not extracted.

mgedmin commented 8 years ago

Correct me if I'm wrong -- on Python 2.6/2.7 builtin functions have both __code__ and func_code as an alias for each other, but Zope's PythonScripts still use only func_code, and this patch works because six._func_code is func_code on 2.x?

I think this corner case ought to get a unit test.

And also PythonScript should be fixed to get __code__/__func__ aliases for forwards-compatibility.

And also the Zope project as a whole needs to make a decision about continuing to support Python 3.2 now that large parts of the ecosystem no longer support it.

Do you want to raise the question of Python 3.2 support on the zope-dev@ mailing list?

mauritsvanrees commented 8 years ago

I am working with Johannes here. The tests pass fine on Python3.2 on my laptop. I see on Travis it goes wrong because the pip version no longer supports 3.2. On a python3.2 prompt u'' gives a SyntaxError and on python3.3 this is accepted.

I will raise this on zope-dev.

mgedmin commented 8 years ago

In theory fixing Travis on Python 3.2 is easy: add a build step that looks like

- if [ $TOXENV = py32 ]; then pip install 'virtualenv<14' 'pip<8'; fi

(for projects that have a matrix based on Python versions instead of TOXENVs the check would be if [ $TRAVIS_PYTHON_VERSION = 3.2 ])

The question is: should we do that (maybe in a for loop for all projects), or should we just remove Python 3.2 support.

Anyway, I want to get that sorted out in master before accepting pull requests, because an unhealthy CI is a sign of an unhealthy project.

thet commented 8 years ago

Still in the pipeline, I'll come back to this soon! Ref for self:

jensens commented 7 years ago

Btw., what is missing here? I do not get it from the discussion above.

mauritsvanrees commented 7 years ago

This is ready for merge as far as I am concerned. Could be squashed, particularly because there were two approaches here. But squashing can be done within the github UI.

mauritsvanrees commented 7 years ago

I managed to add tests. This also showed that we needed a few more changes.

Note that we have prepared changes in CMFCore to no longer need this. But current CMFCore versions would still need it.