Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Py3] allow to run wsgi - part1 #393

Closed
wants to merge 15 commits into from
Closed

[Py3] allow to run wsgi - part1 #393

wants to merge 15 commits into from

Conversation

MartinBasti
Copy link
Contributor

@MartinBasti MartinBasti commented Jan 12, 2017

With these patches we can run commands with server running on py3

Note: to use py3 install module python3-mod_wsgi that enables py3 wsgi automatically

Note: this may or may not depend (I haven't tested) on my PR #382 so it contains all patches, will be rebased when PR #382 merged

first WSGI related commit is

  • py3: session.py decode server name to str … 7ff4b83

@MartinBasti MartinBasti changed the title [WIP] Py3 allow to run wsgi [Py3] allow to run wsgi - part1 Jan 16, 2017
@tiran
Copy link
Member

tiran commented Jan 18, 2017

@MartinBasti cert tests are failing. I have restarted the failing job. Let's see if the error persists or was just caused by a Travis hick up.

@MartinBasti
Copy link
Contributor Author

@tiran we found the issue that caues random test fails, @HonzaCholasta will provide PR with fix, that should be pushed before these commits

This fix is temporal because Memcache will be removed soon, so it is
more workaround than fix

https://fedorahosted.org/freeipa/ticket/4985
Variable 'e' has only local scope in except block in Py3

https://fedorahosted.org/freeipa/ticket/4985
ccache contains binary data, so it should be read and write in binary
mode

https://fedorahosted.org/freeipa/ticket/4985
WSGI prints TypeError into error log when IPA doesn't return bytes in
list as result

https://fedorahosted.org/freeipa/ticket/4985
variable 'e' is valid only in except block in py3, so it must be
assigned to different variable for further usage

https://fedorahosted.org/freeipa/ticket/4985
Update encoding/decoding accordingly to work under Py3

Removing functions that were used only once in code and give no real
improvements

https://fedorahosted.org/freeipa/ticket/4985
Remove decode() as it causes error in py3 because the attribute is
already string not bytes

https://fedorahosted.org/freeipa/ticket/4985
Bytes are unsupported and we should raise a TypeError from Principal
__init__ method otherwise we get hard to debug result
csr must be in string because framework excpects only strings, so we
have to decode it back

https://fedorahosted.org/freeipa/ticket/4985
@MartinBasti
Copy link
Contributor Author

Rebased, ready to be reviewed

@@ -21,6 +21,7 @@
import os
import re
import time
import io
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any particular reason you are using the io module here? In Python 3 io.open is open. In Python 2 the io module's main use is to auto-encode UTF-8 text. It's not wrong, I'm just curious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to use the same

@@ -85,9 +85,16 @@ def strip_header(pem):
"""
Remove the header and footer from a certificate.
"""
s = pem.find("-----BEGIN CERTIFICATE-----")
if isinstance(pem, bytes):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a regular expression?

def strip_header(pem):
    regexp = u"^-----BEGIN CERTIFICATE-----\s+(.*?)\s+^-----END CERTIFICATE-----"
    if isinstance(pem, bytes):
        regexp = regexp.encode('ascii')
    s = re.search(regexp, pem, re.MULTILINE | re.DOTALL)
    if s is not None:
        return s.group(1)
    else:
        return pem

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, thanks

@tiran tiran added the ack Pull Request approved, can be merged label Jan 31, 2017
@MartinBasti MartinBasti added the pushed Pull Request has already been pushed label Jan 31, 2017
@MartinBasti MartinBasti deleted the py3-wsgi branch January 31, 2017 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
2 participants