incubator-airflow git commit: [AIRFLOW-705][AIRFLOW-706] Fix run_command bugs

[xu...@apache.org]

Repository: incubator-airflow
Updated Branches:
  refs/heads/master 6bbf54154 -> 0bb6f2f45


[AIRFLOW-705][AIRFLOW-706] Fix run_command bugs

`run_command` currently uses `str.split` for shell
commands, but this
breaks commands like `echo "foo bar"`.  Instead,
use `shlex.split`,
which understands how to handle quotes.

In python3, subprocess functions return
stdout/stderr as `bytes`, not
`str`.  To fix py2/py3 compatibility,
`run_command` now decodes the
output so that it always returns a `str`.

Dear Airflow Maintainers,

Please accept this PR that addresses the following
issues:
-
https://issues.apache.org/jira/browse/AIRFLOW-705
-
https://issues.apache.org/jira/browse/AIRFLOW-706

Testing Done:
- Changed `sql_alchemy_conn` to
`sql_alchemy_conn_cmd` in test configuration to
exercise `run_command` in tests

Closes #2053 from thesquelched/AIRFLOW-706


Project: http://git-wip-us.apache.org/repos/asf/incubator-airflow/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-airflow/commit/0bb6f2f4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-airflow/tree/0bb6f2f4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-airflow/diff/0bb6f2f4

Branch: refs/heads/master
Commit: 0bb6f2f458ddf305d77fc63d5e817c9b6eb22270
Parents: 6bbf541
Author: Scott Kruger <sc...@chojin.org>
Authored: Thu Mar 16 19:43:01 2017 -0400
Committer: Li Xuanji <xu...@gmail.com>
Committed: Thu Mar 16 19:43:01 2017 -0400

----------------------------------------------------------------------
 airflow/configuration.py |  7 +++++--
 tests/core.py            | 16 +++++++++++++++-
 2 files changed, 20 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/0bb6f2f4/airflow/configuration.py
----------------------------------------------------------------------
diff --git a/airflow/configuration.py b/airflow/configuration.py
index fb3c11e..895c08d 100644
--- a/airflow/configuration.py
+++ b/airflow/configuration.py
@@ -24,6 +24,8 @@ import os
 import six
 import subprocess
 import warnings
+import shlex
+import sys
 
 from future import standard_library
 standard_library.install_aliases()
@@ -76,8 +78,9 @@ def run_command(command):
     Runs command and returns stdout
     """
     process = subprocess.Popen(
-        command.split(), stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    output, stderr = process.communicate()
+        shlex.split(command), stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    output, stderr = [stream.decode(sys.getdefaultencoding(), 'ignore')
+                      for stream in process.communicate()]
 
     if process.returncode != 0:
         raise AirflowConfigException(

http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/0bb6f2f4/tests/core.py
----------------------------------------------------------------------
diff --git a/tests/core.py b/tests/core.py
index e17920a..848553a 100644
--- a/tests/core.py
+++ b/tests/core.py
@@ -60,7 +60,7 @@ from airflow.utils.dates import infer_time_unit, round_time, scale_time_units
 from airflow.utils.logging import LoggingMixin
 from lxml import html
 from airflow.exceptions import AirflowException
-from airflow.configuration import AirflowConfigException
+from airflow.configuration import AirflowConfigException, run_command
 
 import six
 
@@ -1044,6 +1044,20 @@ class CoreTest(unittest.TestCase):
         session.commit()
         session.close()
 
+    def test_run_command(self):
+        if six.PY3:
+            write = r'sys.stdout.buffer.write("\u1000foo".encode("utf8"))'
+        else:
+            write = r'sys.stdout.write(u"\u1000foo".encode("utf8"))'
+
+        cmd = 'import sys; {0}; sys.stdout.flush()'.format(write)
+
+        self.assertEqual(run_command("python -c '{0}'".format(cmd)),
+                         u'\u1000foo' if six.PY3 else 'foo')
+
+        self.assertEqual(run_command('echo "foo bar"'), u'foo bar\n')
+        self.assertRaises(AirflowConfigException, run_command, 'bash -c "exit 1"')
+
 
 class CliTests(unittest.TestCase):
     def setUp(self):