On 14 May 2013 20:02, Helga Velroyen <[email protected]> wrote: > > > > On Tue, May 14, 2013 at 7:26 PM, Helga Velroyen <[email protected]> wrote: >> >> Hi! >> >> >> On Tue, May 14, 2013 at 7:14 PM, Bernardo Dal Seno <[email protected]> >> wrote: >>> >>> On 14 May 2013 18:30, Helga Velroyen <[email protected]> wrote: >>> > This adds functionality to retrieve disk space information >>> > for file storage. It calls the 'df' tool and parses its >>> > output to extract the total and free amount of disk space >>> > on the disk where the given path is located. >>> > The code is not integrated yet, but thoroughly unit-tested. >>> > >>> > Signed-off-by: Helga Velroyen <[email protected]> >>> >>> > diff --git a/lib/storage/filestorage.py b/lib/storage/filestorage.py >>> > new file mode 100644 >>> > index 0000000..47152c8 >>> > --- /dev/null >>> > +++ b/lib/storage/filestorage.py >>> > @@ -0,0 +1,78 @@ >>> > +# >>> > +# >>> > + >>> > +# Copyright (C) 2011, 2012 Google Inc. >>> >>> This should be just 2013. >> >> >> Sure. >> >>> >>> >>> > +def GetSpaceInfo(path, _parsefn=_ParseDfResult): >>> > + """Retrieves the free and total space of the device where the file >>> > is >>> > + located. >>> > + >>> > + @type path: string >>> > + @param path: Path of the file whose embracing device's capacity >>> > is >>> > + reported. >>> > + @type: _parse_fn: function >>> > + @param: _parse_fn: Function that parses the output of the 'df' >>> > command; >>> > + given as parameter to make this code more testable. >>> > + @return: a JSON dictionary containing 'vg_size' and 'vg_free' >>> >>> Why don't you return just a normal dictionary? Where do you plan to use >>> it? >> >> >> In the RPC call NodeInfo. You are right, I guess it should be a normal >> dict at this point. I'll send an interdiff. >> >>> >>> >>> > diff --git a/test/py/ganeti.storage.filestorage_unittest.py >>> > b/test/py/ganeti.storage.filestorage_unittest.py >>> > new file mode 100755 >>> > index 0000000..90d5c26 >>> > --- /dev/null >>> > +++ b/test/py/ganeti.storage.filestorage_unittest.py >>> > @@ -0,0 +1,96 @@ >>> > +#!/usr/bin/python >>> > +# >>> > + >>> > +# Copyright (C) 2006, 2007, 2010, 2012, 2013 Google Inc. >>> >>> Again, just 2013. >> >> >> Sure. >> >>> >>> >>> > + def testParseDfOutputValidInput(self): >>> > + """Tests that parsing of the out put of 'df' works correctly. >>> >>> s/out put/output/ >> >> >> Sure. >> >>> >>> >>> > + def testParseDfOutputInvalidInput(self): >>> > + """Tests that parsing of the out put of 'df' works correctly. >>> >>> s/out put/output/ >> >> >> Sure. >> >>> >>> >>> > +if __name__ == "__main__": >>> > + testutils.GanetiTestProgram() >>> > + >>> > +if __name__ == "__main__": >>> > + testutils.GanetiTestProgram() >>> >>> These lines are needlessly repeated. >> >> >> Oops, true. >> >> Cheers, >> Helga > > > Interdiff: > > diff --git a/lib/storage/filestorage.py b/lib/storage/filestorage.py > index 6080d8a..92cea2e 100644 > --- a/lib/storage/filestorage.py > +++ b/lib/storage/filestorage.py > @@ -1,7 +1,7 @@ > # > # > > -# Copyright (C) 2011, 2012 Google Inc. > +# Copyright (C) 2013 Google Inc. > # > # This program is free software; you can redistribute it and/or modify > # it under the terms of the GNU General Public License as published by > @@ -67,7 +67,7 @@ def GetSpaceInfo(path, _parsefn=_ParseDfResult): > @type _parse_fn: function > @param _parse_fn: Function that parses the output of the 'df' command; > given as parameter to make this code more testable. > - @return: a JSON dictionary containing 'vg_size' and 'vg_free' > + @return: a dictionary containing 'vg_size' and 'vg_free' > """ > cmd = ['df', '-BM', path] > result = utils.RunCmd(cmd) > @@ -75,4 +75,4 @@ def GetSpaceInfo(path, _parsefn=_ParseDfResult): > raise errors.CommandError("Failed to run 'df' command: %s - %s" % > (result.fail_reason, result.output)) > (size, free) = _parsefn(result.stdout) > - return '{"vg_size": %s, "vg_free": %s}' % (size, free) > + return {"vg_size": size, "vg_free": free} > diff --git a/test/py/ganeti.storage.filestorage_unittest.py > b/test/py/ganeti.storage.filestorage_unittest.py > index 90d5c26..03b03b2 100755 > --- a/test/py/ganeti.storage.filestorage_unittest.py > +++ b/test/py/ganeti.storage.filestorage_unittest.py > @@ -1,7 +1,7 @@ > #!/usr/bin/python > # > > -# Copyright (C) 2006, 2007, 2010, 2012, 2013 Google Inc. > +# Copyright (C) 2013 Google Inc. > # > # This program is free software; you can redistribute it and/or modify > # it under the terms of the GNU General Public License as published by > @@ -46,7 +46,7 @@ class TestFileStorageSpaceInfo(unittest.TestCase): > info = filestorage.GetSpaceInfo("/") > > def testParseDfOutputValidInput(self): > - """Tests that parsing of the out put of 'df' works correctly. > + """Tests that parsing of the output of 'df' works correctly. > > """ > valid_df_output = \ > @@ -63,7 +63,8 @@ class TestFileStorageSpaceInfo(unittest.TestCase): > > > def testParseDfOutputInvalidInput(self): > - """Tests that parsing of the out put of 'df' works correctly. > + """Tests that parsing of the output of 'df' works correctly when > invalid > + input is given. > > """ > invalid_output_header_missing = \ > @@ -91,6 +92,3 @@ class TestFileStorageSpaceInfo(unittest.TestCase): > > if __name__ == "__main__": > testutils.GanetiTestProgram() > - > -if __name__ == "__main__": > - testutils.GanetiTestProgram() >
LGTM, thanks. Bernardo
