Merge lp:~reldan/glance/fix_logging_in_swift into lp:~glance-coresec/glance/cactus-trunk

[Eldar Nugaev]

Fix logging in swift

[Jay Pipes (jaypipes)]

thanks, Eldar! lgtm.

review: Approve

[lp:~reldan/glance/fix_logging_in_swift]

108. By Soren Hansen on 2011-04-11

Add the migration sql scripts to MANIFEST.in. The gets them included in not only the tarball, but also by setup.py install.

[Rick Harris (rconradharris)]

Really nice job, Eldar.

> 46 + image_swift = StringIO.StringIO("nevergonnamakeit")
> 47 + options = SWIFT_OPTIONS.copy()
> 48 + del options['swift_store_user']
> 49 + self.assertRaises(BackendException,
> 50 + SwiftBackend.add,
> 51 + 2, image_swift, options)

Might be better to DRY up the code some by adding a helper-method, like:

def assertOptionRequiredForSwift(self, key):
  image_swift = StringIO.StringIO("nevergonnamakeit")
  options = SWIFT_OPTIONS.copy()
  del options[key]
  self.assertRaises(BackendException,
    SwiftBackend.add,
    2, image_swift, options)

Marking as Needs Fixing, but if you don't like the idea, I'd be willing to set this as Approved (since the suggestion is so minor :-)

review: Needs Fixing

[lp:~reldan/glance/fix_logging_in_swift]

109. By Jay Pipes on 2011-04-12

Fix up the way the exception is raised from _safe_kill()... When I "fixed" bug 729726, I mistakenly used the traceback as the message. doh.

[Devin Carlen (devcamcar)]

agree with Jay's suggestion, but I'll approve for now.

review: Approve

[Devin Carlen (devcamcar)]

s/Jay/Rick

[Thierry Carrez (ttx)]

LGTM... Getting late, so i wouldn't be too picky on the test style :)

review: Approve

[Jay Pipes (jaypipes)]

agreed. Rick, I'm pushing this through... we can always refactor the test for DRY later.

[lp:~reldan/glance/fix_logging_in_swift]

110. By Jay Pipes on 2011-04-13

Change parsing of headers to accept 'True', 'on', 1 for boolean truth values.

111. By Eldar Nugaev on 2011-04-13

Fix logging in swift

=== modified file 'glance/store/swift.py'
--- glance/store/swift.py	2011-03-23 14:16:10 +0000
+++ glance/store/swift.py	2011-04-11 16:55:55 +0000
@@ -111,9 +111,9 @@
         # should be a stateful object with options parsed once in
         # a constructor.
         if not auth_address:
-            logger.error(msg)
             msg = ("Could not find swift_store_auth_address in configuration "
                    "options.")
+            logger.error(msg)
             raise glance.store.BackendException(msg)
         else:
             full_auth_address = auth_address
@@ -121,15 +121,15 @@
                 full_auth_address = 'https://' + full_auth_address
 
         if not user:
-            logger.error(msg)
             msg = ("Could not find swift_store_user in configuration "
                    "options.")
+            logger.error(msg)
             raise glance.store.BackendException(msg)
 
         if not key:
-            logger.error(msg)
             msg = ("Could not find swift_store_key in configuration "
                    "options.")
+            logger.error(msg)
             raise glance.store.BackendException(msg)
 
         swift_conn = swift_client.Connection(

=== modified file 'tests/unit/test_swift_store.py'
--- tests/unit/test_swift_store.py	2011-03-23 14:16:10 +0000
+++ tests/unit/test_swift_store.py	2011-04-11 16:55:55 +0000
@@ -324,6 +324,42 @@
                           SwiftBackend.add,
                           2, image_swift, SWIFT_OPTIONS)
 
+    def test_add_no_user(self):
+        """
+        Tests that adding options without user raises
+        an appropriate exception
+        """
+        image_swift = StringIO.StringIO("nevergonnamakeit")
+        options = SWIFT_OPTIONS.copy()
+        del options['swift_store_user']
+        self.assertRaises(BackendException,
+                          SwiftBackend.add,
+                          2, image_swift, options)
+
+    def test_no_key(self):
+        """
+        Tests that adding options without key raises
+        an appropriate exception
+        """
+        image_swift = StringIO.StringIO("nevergonnamakeit")
+        options = SWIFT_OPTIONS.copy()
+        del options['swift_store_key']
+        self.assertRaises(BackendException,
+                          SwiftBackend.add,
+                          2, image_swift, options)
+
+    def test_add_no_auth_address(self):
+        """
+        Tests that adding options without auth address raises
+        an appropriate exception
+        """
+        image_swift = StringIO.StringIO("nevergonnamakeit")
+        options = SWIFT_OPTIONS.copy()
+        del options['swift_store_auth_address']
+        self.assertRaises(BackendException,
+                          SwiftBackend.add,
+                          2, image_swift, options)
+
     def test_delete(self):
         """
         Test we can delete an existing image in the swift store