yellow team mailing list archive

[Francesco Banconi <frankban@xxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/charms/oneiric/buildbot-master/helpers-fixes into lp:~yellow/charms/oneiric/buildbot-master/trunk.

Requested reviews:
  Launchpad Yellow Squad (yellow)

For more details, see:
https://code.launchpad.net/~frankban/charms/oneiric/buildbot-master/helpers-fixes/+merge/92534

- The su context manager no longer fails to restore original user if an exception is raised in `yield`.

- The function `make_charm_config_file` is moved from tests to helpers, since it is used by buildbot-slave charm too.

-- 
https://code.launchpad.net/~frankban/charms/oneiric/buildbot-master/helpers-fixes/+merge/92534
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~frankban/charms/oneiric/buildbot-master/helpers-fixes into lp:~yellow/charms/oneiric/buildbot-master/trunk.

=== modified file 'hooks/helpers.py'
--- hooks/helpers.py	2012-02-09 18:54:03 +0000
+++ hooks/helpers.py	2012-02-10 17:41:59 +0000
@@ -34,6 +34,7 @@
 import re
 import subprocess
 import sys
+import tempfile
 from textwrap import dedent
 import time
 import urllib2
@@ -210,6 +211,16 @@
         return s
 
 
+def make_charm_config_file(charm_config):
+    charm_config_file = tempfile.NamedTemporaryFile()
+    charm_config_file.write(yaml.dump(charm_config))
+    charm_config_file.flush()
+    # The NamedTemporaryFile instance is returned instead of just the name
+    # because we want to take advantage of garbage collection-triggered
+    # deletion of the temp file when it goes out of scope in the caller.
+    return charm_config_file
+
+
 def unit_info(service_name, item_name, data=None):
     if data is None:
         data = yaml.safe_load(run('juju', 'status'))
@@ -334,8 +345,10 @@
     current_home = os.getenv('HOME')
     home = get_user_home(user)
     os.environ['HOME'] = home
-    yield Env(uid, gid, home)
-    os.setegid(os.getgid())
-    os.seteuid(os.getuid())
-    if current_home is not None:
-        os.environ['HOME'] = current_home
+    try:
+        yield Env(uid, gid, home)
+    finally:
+        os.setegid(os.getgid())
+        os.seteuid(os.getuid())
+        if current_home is not None:
+            os.environ['HOME'] = current_home

=== modified file 'tests/buildbot-master.test'
--- tests/buildbot-master.test	2012-02-10 13:10:50 +0000
+++ tests/buildbot-master.test	2012-02-10 17:41:59 +0000
@@ -5,28 +5,16 @@
 from helpers import (
     command,
     encode_file,
-    run,
+    make_charm_config_file,
     unit_info,
     wait_for_page_contents,
     wait_for_unit,
     )
 import os.path
-import tempfile
 import unittest
-import yaml
 
 juju = command('juju')
 
-def make_charm_config_file(charm_config):
-    charm_config_file = tempfile.NamedTemporaryFile()
-    charm_config_file.write(yaml.dump(charm_config))
-    charm_config_file.flush()
-    # The NamedTemporaryFile instance is returned instead of just the name
-    # because we want to take advantage of garbage collection-triggered
-    # deletion of the temp file when it goes out of scope in the caller.
-    return charm_config_file
-
-
 def deploy(charm_config):
     charm_config_file = make_charm_config_file(charm_config)
     juju('deploy', 'buildbot-master', '--config='+charm_config_file.name)
@@ -44,7 +32,7 @@
     def test_deploy(self):
         # See if the charm will actual deploy correctly.  This test may be
         # made redundant by standard charm tests run by some future centralized
-        # charm repository.  
+        # charm repository.
         juju('deploy', 'buildbot-master')
         wait_for_unit('buildbot-master')
         self.assertEqual(unit_info('buildbot-master', 'state'), 'started')