added compatibility for python 2.6 for ssh_read command

[AtlasPilotPuppy]

https://issues.apache.org/jira/browse/SPARK-1990

There were some posts on the lists that spark-ec2 does not work with Python 2.6. In addition, we should check the Python version at the top of the script and exit if it's too old

[AmplabJenkins]

Can one of the admins verify this patch?

[mateiz]

What is the difference between Popen and check_output? Will the Popen hide error codes? We should make sure this doesn't break behavior, and ideally use the same code for both 2.6 and 2.7 if Popen always works.

Also, was this the only change needed for Python 2.6?

[mateiz]

Jenkins, test this please

[AtlasPilotPuppy]

Popen is the old way to do it. You could capture the stderror if you want or just let it raise (default behavior). check_output is more pythonic since python 2.7. 2.6 and 2.7 could both use Popen.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[AmplabJenkins]

Merged build finished. All automated tests passed.

[AmplabJenkins]

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15364/

[mateiz]

Let's make 2.7 also use Popen then. It seems unnecessary to have two code paths here if they both result in the error being printed.

[AtlasPilotPuppy]

Agreed.
On Jun 3, 2014 12:08 PM, "Matei Zaharia" [email protected] wrote:

Let's make 2.7 also use Popen then. It seems unnecessary to have two code
paths here if they both result in the error being printed.

—
Reply to this email directly or view it on GitHub
#941 (comment).

[mateiz]

Also, can you update the title of the pull request to say SPARK-1990 and to match the description in the JIRA? This will simplify bookkeeping later. (I'm assuming this fixes the whole JIRA, otherwise let me know if there are other parts missing.)

[pwendell]

Hey one of they things about check_call is that it would give throw an exception if the suprocess returned nonzero. Does this have the same behavior? This is actually is somewhat important because otherwise the script will happily continue having captured the wrong output and it will downstream throw an error that is really hard to debug.

I wonder if we should just inline the definition of check_output from Python 7. That's what's suggested here:

http://stackoverflow.com/questions/4814970/subprocess-check-output-doesnt-seem-to-exist-python-2-6-5

[AtlasPilotPuppy]

That might actually be a smarter idea. We could just extend subprocess to
add check_all and use that.

On Tue, Jun 3, 2014 at 2:43 PM, Patrick Wendell [email protected]
wrote:

Hey one of they things about check_call is that it would give throw an
exception if the suprocess returned nonzero. Does this have the same
behavior? This is actually is somewhat important because otherwise the
script will happily continue having captured the wrong output and it will
downstream throw an error that is really hard to debug.

I wonder if we should just inline the definition of check_output from
Python 7. That's what's suggested here:

http://stackoverflow.com/questions/4814970/subprocess-check-output-doesnt-seem-to-exist-python-2-6-5

—
Reply to this email directly or view it on GitHub
#941 (comment).

[pwendell]

yeah, but just to be clear, I was suggesting adding a function check_call in our script rather than necessarily extending the subprocess module. I guess maybe we could dyanmically add the method to subprocess as well (?)

[AtlasPilotPuppy]

We could dynamically add the method to subprocess within the scope or
create a function in the scope both of them sound like acceptable solutions
On Jun 3, 2014 2:32 PM, "Patrick Wendell" [email protected] wrote:

yeah, but just to be clear, I was suggesting adding a function check_call
in our script rather than necessarily extending the subprocess module. I
guess maybe we could dyanmically add the method to subprocess as well (?)

—
Reply to this email directly or view it on GitHub
#941 (comment).

[pwendell]

To keep it simple, I'd prefer to just create a new function in the scope.

[AtlasPilotPuppy]

I just created _check_output within the module. Fixes the issue in python 2.6 and 2.7

[AtlasPilotPuppy]

Jenkins, test this please

[pwendell]

Jenkins, test this please.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[AmplabJenkins]

Merged build finished. All automated tests passed.

[AmplabJenkins]

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15842/

[pwendell]

Thanks - I've merged this and back ported it into 0.9 and 1.0

[pwendell]

@anantasty FYI this had a bug in it for which I've added a hot fix. I should have tested this before committing.

[AtlasPilotPuppy]

@pwendell Ouch! my bad. Thanks for the fix.