bootstrap basename can be specified via environment variable

gf-atebbe commented 8 years ago

This is an odd use case, but if you're trying to import mandrel on a virtual machine with your current working directory being a mapped volume from OSX, it fails because OSX is a case-insensitive filesystem. When python is attempting to load the bootstrap module, it finds Mandrel.py in my current working directory (/mnt/hgfs/atebbe1/Documents/code/thingy/) and seems to load that instead of the installed package.

In this case, I am running mandrel-runner from a directory that contains a Mandrel.py bootstrap file

Traceback (most recent call last):
  File "/home/adam/virtualenv/bin/mandrel-runner", line 9, in <module>
    load_entry_point('mandrel==0.2.0', 'console_scripts', 'mandrel-runner')()
  File "build/bdist.linux-x86_64/egg/pkg_resources/__init__.py", line 558, in load_entry_point
  File "build/bdist.linux-x86_64/egg/pkg_resources/__init__.py", line 2682, in load_entry_point
  File "build/bdist.linux-x86_64/egg/pkg_resources/__init__.py", line 2355, in load
  File "build/bdist.linux-x86_64/egg/pkg_resources/__init__.py", line 2361, in resolve
  File "/mnt/hgfs/atebbe1/Documents/code/thingy/mandrel.py", line 1, in <module>
    bootstrap.SEARCH_PATHS.append("/etc/conf")
NameError: name 'bootstrap' is not defined

To get around this, I would propose not naming the bootstrap file Mandrel.py, and adding support for an environment variable to override mandrel.bootstrap.__BOOTSTRAP_BASENAME

ethanrowe commented 8 years ago

So, bootstrap is undefined because from mandrel import bootstrap throws an exception when it fails to locate Mandrel.py?

For clarity, can you show (redacted as appropriate)

The directory where your desired Mandrel.py file lives? (Within your VM, or from whichever environment you're using)
The directory from which you're running mandrel-runner (again, within your VM or whatever environment is applicable)
A sample command-line invocation of mandrel-runner that's giving this exception.

I want to be sure we're talking about the same thing before offering opinions.

gf-atebbe commented 8 years ago

My understanding of what's happening is that when python is attempting to import mandrel, it is looking in the default search path, and finding Mandrel.py. On *nix systems this would be ignored, but since the filesystem on OSX (mounted to linux through a shared volume in the VM) is case-insensitive, it is loading this as the mandrel module being imported. The evidence of this is that python is creating mandrel.pyc when it throws the exception, and in the stack trace it refers to the file being imported as "mandrel.py" (see below for details)

This is my default python search path for reference:

adam@adamdev:/mnt/hgfs/atebbe1/Documents/code/mythingy$ python -c 'import sys; print sys.path'
['', '/mnt/hgfs/atebbe1/Documents/code/python-mandrel', '/usr/lib/python2.7', '/usr/lib/python2.7/plat-x86_64-linux-gnu', '/usr/lib/python2.7/lib-tk', '/usr/lib/python2.7/lib-old', '/usr/lib/python2.7/lib-dynload', '/home/adam/.local/lib/python2.7/site-packages', '/usr/local/lib/python2.7/dist-packages', '/usr/lib/python2.7/dist-packages']
adam@adamdev:/mnt/hgfs/atebbe1/Documents/code/mythingy$ echo $PYTHONPATH

adam@adamdev:/mnt/hgfs/atebbe1/Documents/code/mythingy$

Mandrel v0.2.0 with the expected default Mandrel.py file:

adam@adamdev:/mnt/hgfs/atebbe1/Documents/code/mythingy$ ls -l 
total 68
drwxr-xr-x 1 502 dialout   238 Nov 10 10:18 bootstrap
-rw-r--r-- 1 502 dialout  9244 Dec 16 15:35 CHANGELOG.md
-rwxr-xr-x 1 502 dialout  2285 Nov 20 07:06 deploy.sh
-rw-r--r-- 1 502 dialout  1974 Aug 26 13:20 Dockerfile
drwxr-xr-x 1 502 dialout   510 Nov 16 13:35 awesomeapp
-rw-r--r-- 1 502 dialout   102 Feb 18  2015 Mandrel.py
-rw-r--r-- 1 502 dialout    66 Aug 20 08:17 MANIFEST.in
drwxr-xr-x 1 502 dialout   136 Dec  2 11:49 R_client
-rw-r--r-- 1 502 dialout 16686 Dec  1 15:48 README.md
-rw-r--r-- 1 502 dialout   304 Jan 12 16:11 requirements.txt
drwxr-xr-x 1 502 dialout   340 Sep 23 11:20 scripts
-rw-r--r-- 1 502 dialout  2132 Nov 12 09:17 setup.py
-rw-r--r-- 1 502 dialout     6 Dec 16 15:24 VERSION

adam@adamdev:/mnt/hgfs/atebbe1/Documents/code/mythingy$ python -c 'from mandrel import bootstrap'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "mandrel.py", line 1, in <module>
    bootstrap.SEARCH_PATHS.append("/etc/conf")
NameError: name 'bootstrap' is not defined

Note that after attempting to import bootstrap, python creates mandrel.pyc

adam@adamdev:/mnt/hgfs/atebbe1/Documents/code/mythingy$ ls -l
total 68
...
-rw-r--r-- 1 502 dialout   102 Feb 18  2015 Mandrel.py
-rw-r--r-- 1 502 dialout   233 Jan 13 10:15 mandrel.pyc
...

Mandrel v0.2.1 with the bootstrap file being named bootstrapper.py (import occurs without error)

adam@adamdev:/mnt/hgfs/atebbe1/Documents/code/mythingy$ ls -l
total 68
...
-rw-r--r-- 1 502 dialout   102 Feb 18  2015 bootstrapper.py
...

adam@adamdev:/mnt/hgfs/atebbe1/Documents/code/mythingy$ MANDREL_BOOTSTRAP_NAME=bootstrapper.py python -c 'from mandrel import bootstrap'
adam@adamdev:/mnt/hgfs/atebbe1/Documents/code/mythingy$

An example command for mandrel-runner (being executed from the same directory as used above - /mnt/hgfs/atebbe1/Documents/code/mythingy)

adam@adamdev:/mnt/hgfs/atebbe1/Documents/code/mythingy$ mandrel-runner awesomeapp.run arrays

ethanrowe commented 8 years ago

So, I made some comments regarding testing on the associated PR. And this makes me think that we could address the core issues of both issue #8 and this issue here more easily.

Suppose instead of having one variable for root, and one variable for basename, we instead have one variable (MANDREL_BOOTSTRAP_PATH) that should be the full path to a specific bootstrap file.

If that variable is specified, then you use it as the path to be bootstrapped, which means it doesn't care what the basename is, and the mandrel root will be the dirname, wherever it happens to be.

This covers both issues reasonably well, as it gives you flexibility over things, though it still requires a bootstrap file to exist (the issue #8 guy would free you of that requirement, but it's not a big deal IMO).

Furthermore, the testing is much simpler than when one has to cover all the cases I outlined in the PR:

The variable isn't specified, so it behaves normally
The variable is specified with an absolute path and the file named doesn't exist, so it blows up.
The variable is specified with a relative path and the file named doesn't exist, so it blows up.
The variable is specified with an absolute path and the right file is used and the right config root is determined
The variable is specified with a relative path, and it gets converted to absolute and then has the same behavior as the previous case.

Easier, straightforward, less work, decent value.

gf-atebbe commented 8 years ago

That works for me - I'll try to refactor the PR and update it accordingly. Thanks.

gf-atebbe commented 8 years ago

Sorry I didn't read this carefully enough yesterday. It turns out that not requiring a bootstrap file to exist is actually an important use case for me. So I'm back to the concept of having 2 environment variables: either MANDREL_BOOTSTRAP_NAME and MANDREL_ROOT as originally proposed, or MANDREL_BOOTSTRAP_PATH and some other variable specifying if the bootstrap file can be ignored (MANDREL_IGNORE_BOOTSTRAP?). Either way I think the more complicated testing scenario is likely going to be required. Do you have a preference on this?

ethanrowe commented 8 years ago

I don't have a problem with the idea of the two variables. On Jan 14, 2016 07:03, "Adam Tebbe" notifications@github.com wrote:

Sorry I didn't read this carefully enough yesterday. It turns out that not requiring a bootstrap file to exist is actually an important use case for me. So I'm back to the concept of having 2 environment variables: either MANDREL_BOOTSTRAP_NAME and MANDREL_ROOT as originally proposed, or MANDREL_BOOTSTRAP_PATH and some other variable specifying if the bootstrap file can be ignored (MANDREL_IGNORE_BOOTSTRAP?). Either way I think the more complicated testing scenario is likely going to be required. Do you have a preference on this?

— Reply to this email directly or view it on GitHub https://github.com/ethanrowe/python-mandrel/issues/13#issuecomment-171627154 .

ethanrowe / python-mandrel

bootstrap basename can be specified via environment variable #13