Code review comment for lp://staging/~chad.smith/charms/precise/storage/storage-nfs-python-provider

Revision history for this message
Chad Smith (chad.smith) wrote :

Thanks for the review fcorrea, I resolved [1]

[2] Given all the conversation that we had about separating the storage service from the block storage provider, I think that storing the credentials in a file might not be the best thing to do.
....

I've resolved [2] and dropping writing any persistent files during the mirgration of nova code to the block-storage-broker charm at lp:~chad.smith/charms/precise/block-storage-broker/trunk
So, I'll leave it in this branch as our next branch against this charm will be to remove all nova provider code.

[3]
+current_dir = sys.path[0] # I wonder if we could use os.path.dirname(os.path.realpath(__file__))

Fixed

[4] in hooks/common_util.py Line 157
def _is_ext4_filesystem(device_path): # Maybe we could have something more generic as _assert_fstype(device_path, fstype)?

Fixed

[5] in hooks/common_util.py Line 197
I'm fine leaving the "if provider ==" in there and I don't think that part of the code will grow that fast.
...

Yeah I can only see about 3-4 providers in the near future, when we cross that threshold where handling provider type is an issue, we can make a quick branch to resolve that.

« Back to merge proposal