Ngas lite mods #27
Ngas lite mods #27
Conversation
| buf = read(desc, bufsize) | ||
| logger.debug("Read %d bytes from %r" % (len(buf), source)) |
There was a problem hiding this comment.
These extra logger.debug calls for each read/write operation might end up flooding the logfiles. Maybe leaving the initial "Copying from ..." message could be enough?
| def _read(self, count, **kwargs): | ||
| self._desc.read(count) | ||
| def _read(self, count=4096, **kwargs): | ||
| return self._desc.read(count) |
There was a problem hiding this comment.
Mmmmm, it looks like this missing return was what was causing the odd behavior we saw the other day....
| Returns the location of the directory used by the DALiuGE framework to store | ||
| its logs. If `createIfMissing` is True, the directory will be created if it | ||
| currently doesn't exist | ||
| """ |
There was a problem hiding this comment.
What is this directory intended for? The docstring seems like a copy from the function above. From what I see this is meant to be the default working directory (but can still be overridden in the command line).
| @@ -135,6 +135,22 @@ def test_helloworldapp(self): | |||
| h.execute() | |||
| self.assertEqual(six.b(h.greeting), droputils.allDropContents(b)) | |||
|
|
|||
| def test_ngasio(self): | |||
| nd_in = NgasDROP('HelloWorld.txt', 'HelloWorld.txt') | |||
| nd_in.ngasSrv = 'ngas.ddns.net' | |||
There was a problem hiding this comment.
I guess this is fine for now, but if possible it'd be better to remove this dependency on an external service, as it can cause sporadic test failures. A better alternative would be to run the test against a local NGAS installation, but only if one exists (using @unittest.skipIf or similar).
| value = int(value) | ||
| elif isinstance(obj, dlg_string_param): | ||
| value = kwargs.get(attr_name, obj.default_value) | ||
| if value is not None: | ||
| if value is not None and value is not '': |
There was a problem hiding this comment.
an empty string is a valid string, so maybe in this case it's fine to let it go
| @@ -300,19 +300,19 @@ def getmembers(object, predicate=None): | |||
| for attr_name, obj in getmembers(self, lambda a: not(inspect.isfunction(a) or isinstance(a, property))): | |||
| if isinstance(obj, dlg_float_param): | |||
| value = kwargs.get(attr_name, obj.default_value) | |||
| if value is not None: | |||
| if value is not None and value is not '': | |||
There was a problem hiding this comment.
and value != '', otherwise it's an identity comparison. Likewise for the rest below.
| @@ -89,7 +89,7 @@ def addCommonOptions(parser, defaultPort): | |||
| parser.add_option( "--cwd", action="store_true", | |||
| dest="cwd", help="Short for '-w .'", default=False) | |||
| parser.add_option("-w", "--work-dir", | |||
| help="Working directory, defaults to '/' in daemon mode, '.' in interactive mode", default=None) | |||
| help="Working directory, defaults to '/' in daemon mode, '.' in interactive mode", default=utils.getDlgWorkDir()) | |||
There was a problem hiding this comment.
When the default gets picked, we need to ensure it actually exists; otherwise the NM fails to start. This is probably what's causing the unit tests failures in Travis.
There was a problem hiding this comment.
I would thus rewrite this: I'd keep the None as the default value, and then react to this value being None to actually create the directory and change to it.
| @@ -0,0 +1,594 @@ | |||
| { | |||
There was a problem hiding this comment.
Pushed to the wrong branch? This file doesn't belong here I think.
Mostly fixes to NgasLiteIO, but also some other small changes.