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

Fix subprocess.CalledProcessError on Google App Engine #2860

Merged
merged 1 commit into from Mar 8, 2014
Merged

Fix subprocess.CalledProcessError on Google App Engine #2860

merged 1 commit into from Mar 8, 2014

Conversation

daradib
Copy link
Contributor

@daradib daradib commented Mar 3, 2014

Small bugfix for 1.3.x that sets subprocess.CalledProcessError to None on restrictive environments like Google App Engine. Fixes #1825 which broke after #1857.

Set subprocess.CalledProcessError to None on restrictive environments
like Google App Engine because the exception cannot be raised and there
is no need to catch it. Fixes #1825 which broke after #1857.
@mdboom
Copy link
Member

mdboom commented Mar 3, 2014

👍 Thanks. Looks good, assuming Travis-CI agrees with me ;)

@tacaswell
Copy link
Member

Is there a way to convince travis to run in a restricted environment which mocks GAE?

@mgiuca-google Is it possible to get google to throw some cycles at testing on GAE on a semi-regular basis?

@daradib
Copy link
Contributor Author

daradib commented Mar 3, 2014

@tacaswell You could download and run the GAE SDK in Travis (under before_script in .travis.yml). But right now it needs to be manually modified to use matplotlib in the dev web server bundled with the SDK. @mgiuca-google listed the changes #1823 (not sure if all of it is still necessary with the latest SDK).

Will Google support matplotlib without changes to the SDK in the future?

@mgiuca-google
Copy link
Contributor

Hi all. Sorry, I haven't been keeping up with matplotlib on GAE as I am no longer on the GAE team at Google. I can contact the team if you have any specific requirements.

Testing on the dev appserver should give a reasonably good approximation of the real environment, but (as with everything on GAE), there is no substitute for testing on production. Unfortunately, because mpl has C code, there is no way for an external user to run a new version of it on GAE production. You would have to wait for Google engineers to pull the latest version.

@mgiuca-google Is it possible to get google to throw some cycles at testing on GAE on a semi-regular basis?

The Matplotlib test suite is already part of the GAE test suite, so we will never upgrade to a new version of mpl without first ensuring that all the tests pass. But of course, we test the current version of mpl listed here (at the time of writing, 1.2.0), and I assume you're asking about testing against git HEAD. I think it would be prohibitive for us to set up some automated tests against HEAD (given that there is manual work involved in updating to a new version of matplotlib on GAE). I guess I'm not sure what you want out of this --- is it just ensuring that matplotlib will not break when we get around to updating it on GAE? I guess that since you have no real way of knowing whether it will break on GAE, that's our responsibility when we get around to pulling a new version.

Will Google support matplotlib without changes to the SDK in the future?

That was always the plan. However, since I haven't been maintaining it, I'm not sure if anyone has fixed the dev appserver to run Matplotlib without modifications.

@tacaswell
Copy link
Member

@mgiuca-google That addresses my concerns (I am not super familiar with GAE). I had not fully grasped that the users don't get to install what ever version they want. If there will always be a human at google involved in upgrading the available version on GAE and the test suite gets run that is about the best we can do. It would be nice if we could get the feed back sooner rather than later (and you updated your mpl version more often ;)), but oh well.

@tacaswell tacaswell modified the milestones: v1.4.0, v1.3.x Mar 4, 2014
tacaswell added a commit that referenced this pull request Mar 8, 2014
Fix subprocess.CalledProcessError on Google App Engine
@tacaswell tacaswell merged commit a7d9266 into matplotlib:v1.3.x Mar 8, 2014
@daradib daradib deleted the subprocess-google-app-engine branch April 1, 2014 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants