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

Broken imports on Google App Engine #391

Closed
singhj opened this Issue Feb 19, 2014 · 5 comments

Comments

Projects
None yet
3 participants
@singhj

singhj commented Feb 19, 2014

Apparently commit 3315d5a broke the imports on Google App Engine.

streaming.py imports ssl which is fine everywhere except on Google App Engine where ssl tries to import _ssl. That import does not exist on Google App Engine and thus it fails.

@joshthecoder joshthecoder added this to the 3.0 milestone Feb 20, 2014

@joshthecoder joshthecoder added the Bug label Feb 20, 2014

@joshthecoder

This comment has been minimized.

Show comment
Hide comment
@joshthecoder

joshthecoder Feb 20, 2014

Member

A workaround for now may be to enable the SSL library: https://developers.google.com/appengine/docs/python/sockets/ssl_support

I don't think we need this import and we can probably factor it out in the next release.
I'm not even sure why we try to catch the SSLError specifically in this case.

Member

joshthecoder commented Feb 20, 2014

A workaround for now may be to enable the SSL library: https://developers.google.com/appengine/docs/python/sockets/ssl_support

I don't think we need this import and we can probably factor it out in the next release.
I'm not even sure why we try to catch the SSLError specifically in this case.

@joshthecoder

This comment has been minimized.

Show comment
Hide comment
@joshthecoder

joshthecoder Feb 20, 2014

Member

@Aaron1011 Thoughts? Can we just revert this change for now? I don't recall what value it added.

Member

joshthecoder commented Feb 20, 2014

@Aaron1011 Thoughts? Can we just revert this change for now? I don't recall what value it added.

@singhj

This comment has been minimized.

Show comment
Hide comment
@singhj

singhj Feb 20, 2014

@joshthecoder, @Aaron1011,

Turning on SSL in the Google App Engine environment fixed the broken import.

So I'm thinking the streaming API won't work without sockets turned on anyway, so maybe import ssl should be wrapped in a try/except block that tells App Engine devs to turn on SSL and leave it at that?

I'm OK with this issue being closed, with or without the above-mentioned change. Thanks for your help.

singhj commented Feb 20, 2014

@joshthecoder, @Aaron1011,

Turning on SSL in the Google App Engine environment fixed the broken import.

So I'm thinking the streaming API won't work without sockets turned on anyway, so maybe import ssl should be wrapped in a try/except block that tells App Engine devs to turn on SSL and leave it at that?

I'm OK with this issue being closed, with or without the above-mentioned change. Thanks for your help.

@Aaron1011

This comment has been minimized.

Show comment
Hide comment
@Aaron1011

Aaron1011 Feb 20, 2014

Member

@joshthecoder @singhj: I think we should keep it. It's good to be handling SSL errors, even if they occur infrequently. Having a stream end unexpectedly due to an unhandled error could break people's code.

Member

Aaron1011 commented Feb 20, 2014

@joshthecoder @singhj: I think we should keep it. It's good to be handling SSL errors, even if they occur infrequently. Having a stream end unexpectedly due to an unhandled error could break people's code.

@joshthecoder

This comment has been minimized.

Show comment
Hide comment
@joshthecoder

joshthecoder Feb 21, 2014

Member

Okay closing for now since the enabling SSL seems to fix it.

Member

joshthecoder commented Feb 21, 2014

Okay closing for now since the enabling SSL seems to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment