On Thu, Dec 5, 2013 at 1:41 PM, Thomas Thrainer <[email protected]> wrote:
> True, there you get a fobj with which you are doing something. IMHO, this
> makes it more obvious to the reader that the with actually did something and
> returned something to you.
> Also, the more prominent example is 'with open(...) as f', where it's
> immediately obvious that an 'open' call is issued. In fact, the 'with
> file(...) as fobj' form is even discouraged to use in the Python docs
> (http://docs.python.org/2/library/functions.html?highlight=file#file).
>
> 'with self.volume' doesn't tell the reader, who doesn't know the internals
> of the GlusterVolume class, what's happening. That's why I proposed to
> change it to 'with self.volume.Mount()', which reads much better IMHO (and
> it's not so hard to return a small context object in Mount which unmounts
> once going out of the context).
Okay. I've already merged the changes in my local branch but these
should be the relevant interdiffs:
diff --git a/lib/storage/gluster.py b/lib/storage/gluster.py
index 4e253de..8e588de 100644
--- a/lib/storage/gluster.py
+++ b/lib/storage/gluster.py
@@ -202,10 +202,32 @@ class GlusterVolume(object):
"""Try and mount the volume. No-op if the volume is already mounted.
@raises BlockDeviceError: if the mount was unsuccessful
+
+ @rtype: context manager
+ @return: A simple context monager that lets you use this volume for
+ short lived operations like so::
+
+ with volume.mount():
+ # Do operations on volume
+ # Volume is now unmounted
+
"""
+ class _GlusterVolumeContextManager(object):
+
+ def __init__(self, volume):
+ self.volume = volume
+
+ def __enter__(self):
+ # We're already mounted.
+ return self
+
+ def __exit__(self, *exception_information):
+ self.volume.Unmount()
+ return False # do not swallow exceptions.
+
if self._IsMounted():
- return
+ return _GlusterVolumeContextManager(self)
command = ["mount",
"-t", "glusterfs",
@@ -233,6 +255,8 @@ class GlusterVolume(object):
self.mount_point,
reasons)
+ return _GlusterVolumeContextManager(self)
+
def Unmount(self):
"""Try and unmount the volume.
@@ -251,13 +275,6 @@ class GlusterVolume(object):
logging.warning("Failed to unmount %r from %r: %s",
self, self.mount_point, result.fail_reason)
- def __enter__(self):
- self.Mount()
- return self
-
- def __exit__(self, *exception_information):
- self.Unmount()
-
class GlusterStorage(base.BlockDev):
"""File device using the Gluster backend.
@@ -335,7 +352,7 @@ class GlusterStorage(base.BlockDev):
@return: True if the removal was successful
"""
- with self.volume:
+ with self.volume.Mount():
self.file = FileDeviceHelper(self.full_path)
if self.file.Remove():
self.file = None
@@ -427,7 +444,7 @@ class GlusterStorage(base.BlockDev):
# Possible optimization: defer actual creation to first Attach, rather
# than mounting and unmounting here, then remounting immediately after.
- with volume_obj:
+ with volume_obj.Mount():
FileDeviceHelper.CreateFile(full_path, size, create_folders=True)
return GlusterStorage(unique_id, children, size, params, dyn_params)
--
Raffa Santi
Google Germany GmbH
Dienerstr. 12
80331 München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
