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 from_env to capture only explicitly set values. #136

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Fix from_env to capture only explicitly set values.
Previously from_env also captured Variables defaults which had the
effect of defaulted Variables always over-riding user supplied PexInfo
values.
jsirois committed Jul 23, 2015

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit b93ecd728665bfe016311668ccc052998e536e01
22 changes: 12 additions & 10 deletions pex/pex_info.py
Original file line number Diff line number Diff line change
@@ -88,16 +88,18 @@ def from_json(cls, content):

@classmethod
def from_env(cls, env=ENV):
supplied_env = env.strip_defaults()
zip_safe = None if supplied_env.PEX_FORCE_LOCAL is None else not supplied_env.PEX_FORCE_LOCAL
pex_info = {
'pex_root': env.PEX_ROOT,
'entry_point': env.PEX_MODULE,
'script': env.PEX_SCRIPT,
'zip_safe': not env.PEX_FORCE_LOCAL,
'inherit_path': env.PEX_INHERIT_PATH,
'ignore_errors': env.PEX_IGNORE_ERRORS,
'always_write_cache': env.PEX_ALWAYS_CACHE,
'pex_root': supplied_env.PEX_ROOT,
'entry_point': supplied_env.PEX_MODULE,
'script': supplied_env.PEX_SCRIPT,
'zip_safe': zip_safe,
'inherit_path': supplied_env.PEX_INHERIT_PATH,
'ignore_errors': supplied_env.PEX_IGNORE_ERRORS,
'always_write_cache': supplied_env.PEX_ALWAYS_CACHE,
}
# Filter out empty entries.
# Filter out empty entries not explicitly set in the environment.
return cls(info=dict((k, v) for (k, v) in pex_info.items() if v is not None))

@classmethod
@@ -260,10 +262,10 @@ def update(self, other):
self._distributions.update(other.distributions)
self._requirements.update(other.requirements)

def dump(self):
def dump(self, **kwargs):
pex_info_copy = self._pex_info.copy()
pex_info_copy['requirements'] = list(self._requirements)
return json.dumps(pex_info_copy)
return json.dumps(pex_info_copy, **kwargs)

def copy(self):
return PexInfo(info=self._pex_info.copy())
19 changes: 15 additions & 4 deletions pex/variables.py
Original file line number Diff line number Diff line change
@@ -32,8 +32,9 @@ def iter_help(cls):
variable_type, variable_text = cls.process_pydoc(getattr(value, '__doc__'))
yield variable_name, variable_type, variable_text

def __init__(self, environ=None):
def __init__(self, environ=None, use_defaults=True):
self._environ = environ.copy() if environ is not None else os.environ
self._use_defaults = use_defaults

def copy(self):
return self._environ.copy()
@@ -44,6 +45,9 @@ def delete(self, variable):
def set(self, variable, value):
self._environ[variable] = str(value)

def _defaulted(self, default):
return default if self._use_defaults else None

def _get_bool(self, variable, default=False):
value = self._environ.get(variable)
if value is not None:
@@ -54,10 +58,10 @@ def _get_bool(self, variable, default=False):
else:
die('Invalid value for %s, must be 0/1/false/true, got %r' % (variable, value))
else:
return default
return self._defaulted(default)

def _get_string(self, variable, default=None):
return self._environ.get(variable, default)
return self._environ.get(variable, self._defaulted(default))

def _get_path(self, variable, default=None):
value = self._get_string(variable, default=default)
@@ -70,7 +74,14 @@ def _get_int(self, variable, default=None):
except ValueError:
die('Invalid value for %s, must be an integer, got %r' % (variable, self._environ[variable]))
except KeyError:
return default
return self._defaulted(default)

def strip_defaults(self):
"""Returns a copy of these variables but with defaults stripped.
Any variables not explicitly set in the environment will have a value of `None`.
"""
return Variables(environ=self.copy(), use_defaults=False)

@contextmanager
def patch(self, **kw):
31 changes: 31 additions & 0 deletions tests/test_pex_info.py
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@

from pex.orderedset import OrderedSet
from pex.pex_info import PexInfo
from pex.variables import Variables


def make_pex_info(requirements):
@@ -32,3 +33,33 @@ def test_backwards_incompatible_pex_info():
['world==0.2', False, None],
])
assert pi.requirements == OrderedSet(['hello==0.1', 'world==0.2'])


def assert_same_info(expected, actual):
assert expected.dump(sort_keys=True) == actual.dump(sort_keys=True)


def test_from_empty_env():
environ = Variables(environ={})
info = {}
assert_same_info(PexInfo(info=info), PexInfo.from_env(env=environ))


def test_from_env():
environ = dict(PEX_ROOT='/pex_root',
PEX_MODULE='entry:point',
PEX_SCRIPT='script.sh',
PEX_FORCE_LOCAL='true',
PEX_INHERIT_PATH='true',
PEX_IGNORE_ERRORS='true',
PEX_ALWAYS_CACHE='true')

info = dict(pex_root='/pex_root',
entry_point='entry:point',
script='script.sh',
zip_safe=False,
inherit_path=True,
ignore_errors=True,
always_write_cache=True)

assert_same_info(PexInfo(info=info), PexInfo.from_env(env=Variables(environ=environ)))
17 changes: 17 additions & 0 deletions tests/test_variables.py
Original file line number Diff line number Diff line change
@@ -72,3 +72,20 @@ def test_pex_vars_set():
assert v._get_int('HELLO') == 42
v.delete('HELLO')
assert v._get_int('HELLO') is None


def test_pex_vars_defaults_stripped():
v = Variables(environ={})
stripped = v.strip_defaults()

# bool
assert v.PEX_ALWAYS_CACHE is not None
assert stripped.PEX_ALWAYS_CACHE is None

# string
assert v.PEX_PATH is not None
assert stripped.PEX_PATH is None

# int
assert v.PEX_VERBOSE is not None
assert stripped.PEX_VERBOSE is None