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

Proposed by Eldar Nugaev
Status: Merged
Approved by: Jay Pipes
Approved revision: no longer in the source branch.
Merged at revision: 111
Proposed branch: lp://staging/~reldan/glance/fix_logging_in_swift
Merge into: lp://staging/~glance-coresec/glance/cactus-trunk
Diff against target: 79 lines (+39/-3)
2 files modified
glance/store/swift.py (+3/-3)
tests/unit/test_swift_store.py (+36/-0)
To merge this branch: bzr merge lp://staging/~reldan/glance/fix_logging_in_swift
Reviewer Review Type Date Requested Status
Thierry Carrez (community) Approve
Devin Carlen (community) Approve
Rick Harris (community) Needs Fixing
Jay Pipes (community) Approve
Review via email: mp+57196@code.staging.launchpad.net

Description of the change

Fix logging in swift

To post a comment you must log in.
Revision history for this message
Jay Pipes (jaypipes) wrote :

thanks, Eldar! lgtm.

review: Approve
108. By Soren Hansen

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

Revision history for this message
Rick Harris (rconradharris) wrote :

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
109. By Jay Pipes

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.

Revision history for this message
Devin Carlen (devcamcar) wrote :

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

review: Approve
Revision history for this message
Devin Carlen (devcamcar) wrote :

s/Jay/Rick

Revision history for this message
Thierry Carrez (ttx) wrote :

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

review: Approve
Revision history for this message
Jay Pipes (jaypipes) wrote :

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

110. By Jay Pipes

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

111. By Eldar Nugaev

Fix logging in swift

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches