Juju Charms Collection
storage package

Merge lp://qastaging/~tribaal/charms/precise/storage/make-local-provider-answer into lp://qastaging/charms/storage

  1. Precise Pangolin (12.04)
  2. make-local-provider-answer
  3. Merge into trunk
Proposed by Chris Glass
Status: Merged
Approved by: David Britton
Approved revision: 38
Merged at revision: 37
Proposed branch: lp://qastaging/~tribaal/charms/precise/storage/make-local-provider-answer
Merge into: lp://qastaging/charms/storage
Diff against target: 30 lines (+23/-3)
1 file modified
hooks/storage-provider.d/local/data-relation-changed (+23/-3)
To merge this branch: bzr merge lp://qastaging/~tribaal/charms/precise/storage/make-local-provider-answer
Reviewer Review Type Date Requested Status
Charles Butler (community) Approve
David Britton (community) Approve
Stuart Bishop (community) Approve
Review via email: mp+231065@code.qastaging.launchpad.net

Description of the change

This fixes the related bug. The local storage provider now simply answers the main charm immediately (since no IO is actually necessary).

To post a comment you must log in.
Revision history for this message
David Britton (dpb) wrote :

I tested this with postgresql and noticed that the default charmhelpers mount creation algorithm has a bug with permissions and ownership. I'll file a bug on that separately, but that led me to think that we should be creating the directory in the local provider so it is ready to go for the caller.

a simple 'os.makedirs()' on the mountpoint if it does not exist would be helpful and expected IMO.

Other than this, things look great. I'll mark needs fixing until then.

review: Needs Fixing
Revision history for this message
David Britton (dpb) wrote :

Added: https://bugs.launchpad.net/charm-helpers/+bug/1361866

for the charmhelpers issue.

Revision history for this message
Chris Glass (tribaal) wrote :

Ohhh good catch, thanks. The charm I developed this for already has a mkdirs() call so I didn't see the issue. Will fix.

lp://qastaging/~tribaal/charms/precise/storage/make-local-provider-answer updated
38. By Chris Glass

Added os.makedirs() call on the mountpoint to make sure it exists.

Revision history for this message
Chris Glass (tribaal) wrote :

Should be fixed now. Let me know if anything else is necessary.

Revision history for this message
Stuart Bishop (stub) wrote :

Looks good.

review: Approve
Revision history for this message
David Britton (dpb) :
review: Approve
Revision history for this message
Charles Butler (lazypower) wrote :

Worked as advertised. LGTM +1

review: Approve

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

to all changes: