Permalink
Browse files

Combine PYTHONPATH with local path separator when running jobs locally

  • Loading branch information...
1 parent d4329cf commit becf00152b5de4106be420c882354a868d0b5a51 David Marin committed Nov 12, 2010
Showing with 55 additions and 7 deletions.
  1. +1 −0 docs/configs-conf.rst
  2. +1 −1 docs/configs-reference.rst
  3. +15 −1 mrjob/conf.py
  4. +9 −2 mrjob/local.py
  5. +5 −3 mrjob/runner.py
  6. +24 −0 tests/conf_test.py
@@ -19,6 +19,7 @@ Combiner functions take a list of values to combine, with later options taking p
.. autofunction:: combine_lists
.. autofunction:: combine_dicts
.. autofunction:: combine_envs
+.. autofunction:: combine_local_envs
.. autofunction:: combine_paths
.. autofunction:: combine_path_lists
@@ -38,7 +38,7 @@ Option Default Combined by
See :py:meth:`mrjob.runner.MRJobRunner.__init__` for details.
-:py:class:`~mrjob.local.LocalMRJobRunner` takes no additional options.
+:py:class:`~mrjob.local.LocalMRJobRunner` takes no additional options, but *cmdenv* is combined with :py:func:`~mrjob.conf.combine_local_envs` instead.
Additional options for :py:class:`~mrjob.emr.EMRJobRunner`
----------------------------------------------------------
View
@@ -257,12 +257,26 @@ def combine_envs(*envs):
If you pass in ``None`` in place of a dictionary, it will be ignored.
"""
+ return _combine_envs_helper(envs, local=False)
+
+def combine_local_envs(*envs):
+ """Same as :py:func:`combine_envs`, except that paths are combined
+ using the local path separator (e.g ``;`` on Windows rather than ``:``).
+ """
+ return _combine_envs_helper(envs, local=True)
+
+def _combine_envs_helper(envs, local):
+ if local:
+ pathsep = os.pathsep
+ else:
+ pathsep = ':'
+
result = {}
for env in envs:
if env:
for key, value in env.iteritems():
if key.endswith('PATH') and result.get(key):
- result[key] = '%s:%s' % (value, result[key])
+ result[key] = value + pathsep + result[key]
else:
result[key] = value
View
@@ -22,7 +22,7 @@
import sys
import re
-from mrjob.conf import combine_envs
+from mrjob.conf import combine_dicts, combine_local_envs
from mrjob.parse import find_python_traceback, parse_mr_job_stderr
from mrjob.runner import MRJobRunner
from mrjob.util import cmd_line, file_ext, unarchive
@@ -53,6 +53,13 @@ def __init__(self, **kwargs):
self._final_outfile = None
self._counters = {}
+ @classmethod
+ def _opts_combiners(cls):
+ # on windows, PYTHONPATH should use ;, not :
+ return combine_dicts(
+ super(LocalMRJobRunner, cls)._opts_combiners(),
+ {'cmdenv': combine_local_envs})
+
def _run(self):
if self._opts['bootstrap_mrjob']:
self._add_python_archive(self._create_mrjob_tar_gz() + '#')
@@ -167,7 +174,7 @@ def _invoke_step(self, args, outfile_name, env=None):
"""
# keep the current environment because we need PATH to find binaries
# and make PYTHONPATH work
- env = combine_envs(
+ env = combine_local_envs(
{'PYTHONPATH': os.getcwd()},
os.environ,
self._cmdenv,
View
@@ -33,7 +33,7 @@
except ImportError:
from StringIO import StringIO
-from mrjob.conf import combine_dicts, combine_envs, combine_lists, combine_opts, combine_paths, combine_path_lists, load_opts_from_mrjob_conf
+from mrjob.conf import combine_dicts, combine_envs, combine_local_envs, combine_lists, combine_opts, combine_paths, combine_path_lists, load_opts_from_mrjob_conf
from mrjob.util import cmd_line, file_ext, tar_and_gzip
log = logging.getLogger('mrjob.runner')
@@ -556,7 +556,9 @@ def _add_archive_for_upload(self, path):
def _add_python_archive(self, path):
file_dict = self._add_archive_for_upload(path)
log.debug('adding %s to PYTHONPATH' % file_dict['name'])
- self._cmdenv = combine_envs(
+ # on Windows, PYTHONPATH should be separated by ;, not :
+ cmdenv_combiner = self._opts_combiners()['cmdenv']
+ self._cmdenv = cmdenv_combiner(
self._cmdenv, {'PYTHONPATH': file_dict['name']})
self._python_archives.append(file_dict)
@@ -666,7 +668,7 @@ def _get_steps(self):
self._mr_job_extra_args(local=True))
log.debug('> %s' % cmd_line(args))
# add . to PYTHONPATH (in case mrjob isn't actually installed)
- env = combine_envs(os.environ,
+ env = combine_local_envs(os.environ,
{'PYTHONPATH': os.path.abspath('.')})
steps_proc = Popen(args, stdout=PIPE, stderr=PIPE, env=env)
stdout, stderr = steps_proc.communicate()
View
@@ -224,6 +224,30 @@ def test_paths(self):
'CLASSPATH': '/home/dave/java',
'PS1': '\w> '})
+class CombineLocalEnvsTestCase(TestCase):
+
+ @setup
+ def set_os_pathsep(self):
+ self._real_os_pathsep = os.pathsep
+ os.pathsep = ';'
+
+ @teardown
+ def restore_os_pathsep(self):
+ os.pathsep = self._real_os_pathsep
+
+ def test_paths(self):
+ assert_equal(combine_local_envs(
+ {'PATH': '/bin:/usr/bin',
+ 'PYTHONPATH': '/usr/lib/python/site-packages',
+ 'PS1': '> '},
+ {'PATH': '/home/dave/bin',
+ 'PYTHONPATH': '/home/dave/python',
+ 'CLASSPATH': '/home/dave/java',
+ 'PS1': '\w> '}),
+ {'PATH': '/home/dave/bin;/bin:/usr/bin',
+ 'PYTHONPATH': '/home/dave/python;/usr/lib/python/site-packages',
+ 'CLASSPATH': '/home/dave/java',
+ 'PS1': '\w> '})
class CombineListsTestCase(TestCase):

0 comments on commit becf001

Please sign in to comment.