Re: [PATCH master 05/14] FileStorage: extract file logic to a FileDeviceHelper object

[Santi Raffa]

On Thu, Dec 5, 2013 at 1:22 PM, Thomas Thrainer <thoma...@google.com> wrote:
> On Thu, Dec 5, 2013 at 10:19 AM, Santi Raffa <rsa...@google.com> wrote:
>> +    if not _file_path_acceptance_fn:
>> +      _file_path_acceptance_fn = CheckFileStoragePathAcceptance
>> +    _file_path_acceptance_fn(path)
>> +
>
>
> I think it'd be cleaner to put this as the default value in the parameter
> list.

I tried that, but it's not possible because this is a top-level
definition that comes before CheckFileStoragePathAcceptance's; trying
to do that gets you:

NameError: name 'CheckFileStoragePathAcceptance' is not defined

>> -  def Grow(self, amount):
>> +  def Grow(self, amount, dryrun, backingstore, excl_stor):
>
>
> Why excl_stor?

If we're going to modify the implementation of FileDeviceHelper.Grow
to handle the disk templates' parameters uniformly, I figured we
might've as well passed all of the parameters to it.

OTOH, changing the signature of Grow and the name of Create requires
changing the tests to match.

--- a/test/py/ganeti.storage.filestorage_unittest.py
+++ b/test/py/ganeti.storage.filestorage_unittest.py
@@ -227,7 +227,7 @@ class TestFileDeviceHelper(testutils.GanetiTestCase):
   def _Make(path, create_with_size=None, create_folders=False):
     skip_checks = lambda path: None
     if create_with_size:
-      return filestorage.FileDeviceHelper.Create(
+      return filestorage.FileDeviceHelper.CreateFile(
         path, create_with_size, create_folders=create_folders,
         _file_path_acceptance_fn=skip_checks
       )
@@ -251,7 +251,7 @@ class TestFileDeviceHelper(testutils.GanetiTestCase):
     should_fail( lambda: \
       TestFileDeviceHelper._Make(path).Size())
     should_fail( lambda: \
-      TestFileDeviceHelper._Make(path).Grow(20))
+      TestFileDeviceHelper._Make(path).Grow(20, True, False, None))

     # Removing however fails silently.
     TestFileDeviceHelper._Make(path).Remove()
@@ -278,9 +278,9 @@ class TestFileDeviceHelper(testutils.GanetiTestCase):
       TestFileDeviceHelper._Make(path).Exists(assert_exists=False))

     should_fail( lambda: \
-      TestFileDeviceHelper._Make(path).Grow(-30))
+      TestFileDeviceHelper._Make(path).Grow(-30, True, False, None))

-    TestFileDeviceHelper._Make(path).Grow(58)
+    TestFileDeviceHelper._Make(path).Grow(58, True, False, None)
     self.assertEqual(100 * 1024 * 1024,
                      TestFileDeviceHelper._Make(path).Size())

-- 
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