Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Work around missing subprocess members on Google App Engine#1825
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Work around missing subprocess members on Google App Engine#1825
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| a2 = Popen('ps -p %d -o osz' % pid, shell=True, | ||
| stdout=PIPE).stdout.readlines() | ||
| except OSError: | ||
| raise NotImplementedError( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm tempted to suggest a RuntimeError here. None-the-less, this is an API change which should be documented indocs/api/api_changes.rst
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Done. Note that I am having trouble building the docs locally, so I'm hoping my ReST syntax is correct!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Also, I agreeRuntimeError is more appropriate (sinceNotImplementedError implies that it is planning to be fixed one day), but I changed this to be compatible with the Windows implementation. Happy to change it if you want. For what it's worth,NotImplementedError is a subclass ofRuntimeError.
Again, this is looking good@mgiuca-google - thanks for your hard work. |
Hmm ... looks like Travis failed on Python 3 with this error: I'm not sure why my code I think this can be worked around using |
Python 2's default is to use relative imports before absolute ones, so In order to get the behaviour you desire, I think you can just add the line It does look a little like 2to3 is being a little over zealous here... |
…ubprocess.cbook: Moved check_output to subprocess_fixed.backend_pgf: Import check_output from subprocess_fixed instead of cbook.
If subprocess.Popen is missing (for example, on Google App Engine), replaces itwith a dummy version that raises OSError. In these environments, calls tosubprocess will be handled properly as if the called app could not be found,instead of raising AttributeError.
- The subtle change to cbook.report_memory's exception types.- Moving subprocess_fixed to compat.subprocess.
This prevents the module from importing itself (as opposed to the globalsubprocess module) on Python 3 after 2to3 conversion.
OK thanks for the explanation. What I don't get is if Python 2 has relative imports by default, why does it work? (It only stopped working in Python 3.) Oh well, I've added the absolute import so we'll see what Travis says. Also rebased. |
Work around missing subprocess members on Google App Engine
Looks like70f2296 added the line |
@daradib: If you're a Google App Engine user, would you mind writing up a PR that works around the issue? |
On Google App Engine, the
subprocessmodule exists, but is empty. This patch introduces a replacement module,subprocess_fixed, which provides a stub forsubprocess.Popenwhich is compatible with existing usage.I noticed that
cbookalready has a bit of a hack to work around the fact thatsubprocess.check_outputis missing in Python 2.6, so I've moved that into mysubprocess_fixedmodule as well, so now it has two reasons to exist.Calls to
subprocess_fixed.Popenon Google App Engine always raiseOSError, which most of the codebase is already prepared for (since that's what happens when the application is missing). I've added a few missing catches ofOSError.Partial fix for Issue#1823.