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.

Sign up for GitHub

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

Jump to bottom

Add support for environment variables #14

Open
wants to merge 3 commits into
base: release
Choose a base branch
from
Open

Add support for environment variables #14

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
128c142
Support environment variables in bootstrap
gf-atebbe Jan 12, 2016
f6c3b5e
Fixing unit test for python 2.6
gf-atebbe Jan 13, 2016
dd5ffda
Adding more unit tests to address concerns raised in the PR
gf-atebbe Mar 9, 2016
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
7 changes: 6 additions & 1 deletion changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
2.0.0
0.2.1

* Issue #8: Allow mandrel root to be specified via an environment variable
* Issue #13: Allow bootstrap basename to be specified via an environment variable

0.2.0

* mandrel-runner uses return value of callable as the exit code.

Expand Down
25 changes: 15 additions & 10 deletions mandrel/bootstrap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from mandrel import exception
from mandrel import util

__BOOTSTRAP_BASENAME = 'Mandrel.py'
__BOOTSTRAP_BASENAME = os.getenv('MANDREL_BOOTSTRAP_NAME', 'Mandrel.py')

LOGGING_CONFIG_BASENAME = 'logging.cfg'
DEFAULT_LOGGING_LEVEL = logging.INFO
Expand Down Expand Up @@ -105,12 +105,16 @@ def get_logger(name=None):


def _find_bootstrap_base():
current = os.path.realpath('.')
while not os.path.isfile(os.path.join(current, __BOOTSTRAP_BASENAME)):
parent = os.path.dirname(current)
if parent == current:
raise exception.MissingBootstrapException, 'Cannot find %s file in directory hierarchy' % __BOOTSTRAP_BASENAME
current = parent
if os.getenv('MANDREL_ROOT'):
current = os.getenv('MANDREL_ROOT')
else:
current = os.path.realpath('.')

while not os.path.isfile(os.path.join(current, __BOOTSTRAP_BASENAME)):
parent = os.path.dirname(current)
if parent == current:
raise exception.MissingBootstrapException, 'Cannot find %s file in directory hierarchy' % __BOOTSTRAP_BASENAME
current = parent

return current, os.path.join(current, __BOOTSTRAP_BASENAME)

Expand All @@ -129,9 +133,10 @@ def parse_bootstrap_file():
This makes it easy for the BOOTSTRAP_FILE to configure mandrel settings
without performing further imports.
"""
with open(BOOTSTRAP_FILE, 'rU') as source:
code = compile(source.read(), BOOTSTRAP_FILE, 'exec')
eval(code, {'bootstrap': sys.modules[__name__], 'config': config})
if os.path.exists(BOOTSTRAP_FILE):
with open(BOOTSTRAP_FILE, 'rU') as source:
code = compile(source.read(), BOOTSTRAP_FILE, 'exec')
eval(code, {'bootstrap': sys.modules[__name__], 'config': config})

(ROOT_PATH, BOOTSTRAP_FILE) = _find_bootstrap_base()
SEARCH_PATHS = util.TransformingList(normalize_path)
Expand Down
10 changes: 10 additions & 0 deletions mandrel/test/bootstrap/bootstrap_file_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import unittest
import mandrel.exception
from mandrel.test import utils
from test.test_support import EnvironmentVarGuard

class TestBootstrapFile(utils.TestCase):
def testNoFile(self):
Expand Down Expand Up @@ -43,3 +44,12 @@ def testDefaultLoggingConfig(self):
utils.refresh_bootstrapper()
self.assertEqual('logging.cfg', mandrel.bootstrap.LOGGING_CONFIG_BASENAME)

def testOSEnv(self):
with EnvironmentVarGuard() as env:
env.set('MANDREL_ROOT', '/blah')
env.set('MANDREL_BOOTSTRAP_NAME', 'bootstrapper.py')

utils.refresh_bootstrapper()
self.assertEqual(os.getenv('MANDREL_ROOT'), mandrel.bootstrap.ROOT_PATH)
expected = os.path.join(os.getenv('MANDREL_ROOT'), os.getenv('MANDREL_BOOTSTRAP_NAME'))
self.assertEqual(expected, mandrel.bootstrap.BOOTSTRAP_FILE)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to see tests, but this changes too much core stuff for a single test case to cover.

Given that you're introducing two changes (the mandrel root variable; the bootstrap basename variable), the cases to consider are considerably more complicated:

  • Base case: neither variable is defined (the original behaviors)
  • Root is specified, bootstrapper is not, Mandrel.py is present in specified root (should parse the file it finds)
  • Root is specified, bootstrapper is not, Mandrel.py is not present in specified root (should get basic defaults)
  • Root is specified, bootstrapper is specified, the specified file exists in the specified root
  • Root is specified, bootstrapper is specified, the specified file does not exist in the specified root
  • Root is not specified, bootstrapper is specified, the specified file exists; it should be used and the root should be where that guy was found. This would need testing at various file system levels.
  • Root is not specified, bootstrapper is specified, the specified file does not exist within the file system hierarchy; this should blow up, just like it does now if Mandrel.py can't be found.

So there's quite a few test cases that would need to be covered.

It's considerably simpler if only one of the issues is done: either specifying a root, or specifying a basename, but not both.

But as it stands, the test only covers the case where both are set, it doesn't treat them separately.

2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

setup(
name = "mandrel",
version = "0.2.0",
version = "0.2.1",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't include this in the PR; the version bump and release gets determined upon or following merge (and in this case will likely result in a minor increment, since we aren't at 1.x yet and this is a significant public interface change).

author = "Ethan Rowe",
author_email = "ethan@the-rowes.com",
description = ("Provides bootstrapping for sane configuration management"),
Expand Down