Merge lp:~salgado/offspring/path-independence into lp:offspring

[Guilherme Salgado]

This is a second attempt at trying to get these changes merged; they simplify things a lot for developers.

Here's what Michael said in the original merge proposal:

This finally is an I think ready branch to allow offspring to be easily run from places other than /srv.

The code to find offspring-builder-config is a touch horrible, but it won't be needed in the slave.

[Cody A.W. Somerville (cody-somerville)]

Branch needs to be updated to use final solution accepted as part of all-python-config branch MPs,

review: Needs Fixing

[Guilherme Salgado (salgado)]

Ok, I've updated it.

[Cody A.W. Somerville (cody-somerville)]

First off, kudos to both you and Michael for working on this. Making it easier to run Offspring from locations other than /srv/ is valuable for a number of reasons including making it easier for developers to jump in and contribute (e.g. you've specifically mentioned making it easier to run tests for a branch as a motivation for pushing this change). I apologize for how long it has taken to get feedback on this branch. The problem is unfortunately not really an easy one to solve elegantly. I've very much wanted to find a way that I could accept the solution you've came up with but I've ultimately came to the conclusion that it is insufficient.

The one benefit the current rigidity provides is the deterministic nature in which Offspring will search for its configuration files independently of how we have Offspring installed. The tact this MP takes is to start making assumptions about how Offspring is installed and the relativity of assets, a coupling we currently do not have. This is at the very least a pragmatic approach to the problem since it is highly unlikely that anyone is installing Offspring in any other way sufficiently unique to cause a problem but it also doesn't seem like we're far off from wanting to support such scenarios (e.g. Debian packaging). Whereas currently such a scenario is reasonably possible, this change introduces assumptions that are directly incompatible to that objective. Additionally, the assumptions made in the bash scripts create a dependency on the location of the scripts within the codebase itself - it would not be immediately obvious that both offspring-build and archive-builds would now depend on being five directories beneath the assumed base directory (see 'for i in 1 2 3 4 5' loops). Thus it is almost ironic, and I say this only in good jest, that this branch is called 'path-independence'. ;)

The ability to set an environmental variable to override the location of the Offspring configuration file is entirely removed in this branch which I think has been a relatively easy to use (though not entirely effective in all scenarios, at least without some of the other changes you make in this branch) solution to this problem though not as 'automatic'. Most *nix programs and scripts look for their assets in a few different locations with each location checked before the next having a higher precedence to be used. It is also very common for an argument to be available to set the location of assets or configuration file(s). And lastly, it is also very common for applications to be customized/modified at build and/or install time to look in the locations desired by the operator. In Offspring itself, setuptools creates stub scripts that are basically hard coded to execute the real code since setuptools is privy to the absolute location of where it installs our python modules. I think we need to look for a solution that leverages some of these techniques to make it easier to run Offspring both from a branch as is desired but in a fashion that is not incompatible with Offspring being installed in a more traditional (and subsequently Debian policy compliant) fashion.

I'll be happy to discuss this with you further once...

review: Disapprove

[David Murphy (schwuk)]

The review of this branch is ongoing.

=== modified file 'config/offspring.cfg'
--- config/offspring.cfg	2011-11-11 01:41:58 +0000
+++ config/offspring.cfg	2011-11-18 14:41:20 +0000
@@ -7,7 +7,15 @@
 #===============================================================================
 # Offspring Global Defaults
 #
+# Some options in this section are given defaults by code so that they
+# can be referenced in subsitutions:
+#
 # base_dir: Base directory of offspring install
+# hostname: The hostname of the running machine
+# username: The name of the user running offspring
+#
+# The other options below are set normally:
+#
 # bin_dir: Directory to offspring binaries
 # config_dir: Directory to offspring configuration files
 # admin_email: E-mail admin notifications should be sent to by default
@@ -21,7 +29,6 @@
 #===============================================================================
 
 [DEFAULT]
-base_dir: /srv
 bin_dir: %(base_dir)s/bin
 config_dir: %(base_dir)s/config
 log_dir: %(base_dir)s/logs
@@ -140,6 +147,7 @@
 poll_frequency: 20
 logfile: %(log_dir)s/offspring-master.log
 launchpad_oauth: %(config_dir)s/launchpad.oauth
+result_directory: %(base_dir)s/builds
 
 #===============================================================================
 # Offspring publisher
@@ -159,8 +167,8 @@
 
 [publisher]
 db: postgres://offspring:temp1234@localhost/offspring
-development_pool: /srv/builds/
-production_pool: /srv/partners/
+development_pool: %(base_dir)s/builds/
+production_pool: %(base_dir)s/partners/
 signer_directory: 
 signer_files: 
 signer_results: 

=== modified file 'lib/offspring/__init__.py'
--- lib/offspring/__init__.py	2010-11-29 08:27:24 +0000
+++ lib/offspring/__init__.py	2011-11-18 14:41:20 +0000
@@ -1,4 +1,9 @@
 # Copyright 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+import os
+
 __version__ = '0.0.1'
+
+offspring_root = os.path.abspath(
+    os.path.join(os.path.dirname(__file__), '..', '..'))

=== modified file 'lib/offspring/build/bin/offspring-build'
--- lib/offspring/build/bin/offspring-build	2011-11-11 01:25:27 +0000
+++ lib/offspring/build/bin/offspring-build	2011-11-18 14:41:20 +0000
@@ -6,17 +6,13 @@
 set -e
 
 _Initialize() {
-    if [ -z "${OFFSPRING_CONFIGSCRIPT}" ]
-    then
-        OFFSPRING_CONFIGSCRIPT="/srv/scripts/offspring-builder-config"
-    fi
-
-    if [ ! -x "${OFFSPRING_CONFIGSCRIPT}" ]
-    then
-        printf "Could not find config script at ${OFFSPRING_CONFIGSCRIPT}\n" >& 2
-        exit 1
-    fi
-
+    SCRIPT_PATH="$(readlink -e "${0}" || which "${0}")"
+    OFFSPRING_ROOT=$SCRIPT_PATH
+    for i in 1 2 3 4 5;
+    do
+        OFFSPRING_ROOT="$(dirname "${OFFSPRING_ROOT}")"
+    done
+    OFFSPRING_CONFIGSCRIPT=${OFFSPRING_ROOT}/scripts/offspring-builder-config
     eval $("${OFFSPRING_CONFIGSCRIPT}")
 
     if [ "$?" -ne "0" ]

=== modified file 'lib/offspring/build/scripts/archive-builds'
--- lib/offspring/build/scripts/archive-builds	2011-11-11 01:25:27 +0000
+++ lib/offspring/build/scripts/archive-builds	2011-11-18 14:41:20 +0000
@@ -8,17 +8,13 @@
 # XXX: This script needs to be rewritten to use new framework.
 #
 
-if [ -z "${OFFSPRING_CONFIGSCRIPT}" ]
-then
-    OFFSPRING_CONFIGSCRIPT="/srv/scripts/offspring-builder-config"
-fi
-
-if [ ! -x "${OFFSPRING_CONFIGSCRIPT}" ]
-then
-    printf "Could not find config script at ${OFFSPRING_CONFIGSCRIPT}\n" >& 2
-    exit 1
-fi
-
+SCRIPT_PATH="$(readlink -e "${0}" || which "${0}")"
+OFFSPRING_ROOT=$SCRIPT_PATH
+for i in 1 2 3 4 5;
+do
+    OFFSPRING_ROOT="$(dirname "${OFFSPRING_ROOT}")"
+done
+OFFSPRING_CONFIGSCRIPT="${OFFSPRING_ROOT}/scripts/offspring-builder-config"
 eval $("${OFFSPRING_CONFIGSCRIPT}")
 
 if [ "$?" -ne "0" ]

=== modified file 'lib/offspring/config.py'
--- lib/offspring/config.py	2011-11-11 01:13:02 +0000
+++ lib/offspring/config.py	2011-11-18 14:41:20 +0000
@@ -8,16 +8,23 @@
 import os
 import sys
 
+from offspring import offspring_root
+
 config = None
 
 def _read_config():
     global config
     config = SafeConfigParser()
+    hostname = os.popen('hostname').read().strip()
+    config.set('DEFAULT', 'base_dir', offspring_root)
+    config.set('DEFAULT', 'hostname', hostname)
+    config.set('DEFAULT', 'username', os.environ['USERNAME'])
 
     if os.environ.has_key("OFFSPRING_CONFIGFILE"):
         CONFIGFILE_PATH = os.environ["OFFSPRING_CONFIGFILE"]
     else:
-        CONFIGFILE_PATH = "/srv/config/offspring.cfg"
+        CONFIGFILE_PATH = os.path.join(
+            offspring_root, 'config', 'offspring.cfg')
     configFile = None
     try:
         configFile = open(CONFIGFILE_PATH)
@@ -27,9 +34,6 @@
             % (CONFIGFILE_PATH, e) )
         sys.exit(1)
     else:
-        hostname = os.popen('hostname').read().strip()
-        config.set('DEFAULT', 'hostname', hostname)
-        config.set('DEFAULT', 'username', os.environ['USERNAME'])
         config.read([CONFIGFILE_PATH + '.' + hostname])
     finally:
         if configFile is not None:

=== modified file 'lib/offspring/web/queuemanager/models.py'
--- lib/offspring/web/queuemanager/models.py	2011-09-27 18:42:21 +0000
+++ lib/offspring/web/queuemanager/models.py	2011-11-18 14:41:20 +0000
@@ -19,6 +19,7 @@
     models
 )
 
+from offspring import config
 from offspring.enums import ProjectBuildStates
 
 
@@ -303,7 +304,7 @@
     def result_directory(self):
         if self.name:
             try:
-                base_dir = getattr(settings, 'BUILDRESULTS_DIRECTORY', '/srv/builds/')
+                base_dir = config.master('result_directory')
                 buildDate, buildId = self.name.rsplit('-', 1)
             except:
                 return None

=== modified file 'lib/offspring/web/settings_production.py'
--- lib/offspring/web/settings_production.py	2010-11-29 08:27:24 +0000
+++ lib/offspring/web/settings_production.py	2011-11-18 14:41:20 +0000
@@ -16,6 +16,3 @@
 EMAIL_HOST = 'localhost'
 
 MEDIA_URL = 'https://offspring.com/assets/'
-
-# Path to build results
-BUILDRESULTS_DIRECTORY = '/srv/offspring.com/www/builds/'