DM-7900: Implement --batch-type None to run the job in the current process #6
Conversation
| @@ -262,7 +262,9 @@ def submitCommand(self, scriptName): | |||
| return "exec %s" % scriptName | |||
|
|
|||
|
|
|||
| BATCH_TYPES = {'pbs': PbsBatch, | |||
| BATCH_TYPES = {'none' : None, | |||
| 'None' : None, | |||
There was a problem hiding this comment.
Instead of supporting both none and None, might we always convert the key to lower case?
There was a problem hiding this comment.
I could do that, but wouldn't I also have to implement case-insensitive argument parsing for the validate? I've done that elsewhere, but it's messy.
There was a problem hiding this comment.
OK, don't worry about it then.
| @@ -334,6 +336,10 @@ def makeBatch(self, args): | |||
| 'submit': 'batchSubmit', | |||
| 'options': 'batchOptions', | |||
| } | |||
|
|
|||
| if not BATCH_TYPES[args.batchType]: | |||
There was a problem hiding this comment.
Should compare against None.
There was a problem hiding this comment.
Is this a stylistic issue or a functional one?
There was a problem hiding this comment.
Maybe it's not necessary. PEP8 says:
Comparisons to singletons like None should always be done with is or is not , never the equality operators.
But you're not using an equality operator. I guess I was just expressing my personal preference, so feel free to ignore me.
There was a problem hiding this comment.
Well, PEP8 also says
Also, beware of writing if x when you really mean if x is not None -- e.g. when testing whether a variable or argument that defaults to None was set to some other value. The other value might have a type (such as a container) that could be false in a boolean context!
so I guess I'll change it
|
|
||
| sys.argv = argv | ||
|
|
||
| cls.parseAndRun() |
There was a problem hiding this comment.
return cls.parseAndRun()
| while i < len(sys.argv): | ||
| a = sys.argv[i] | ||
| if a.startswith("--batch-t"): # i.e. --batch-type and abbreviations thereof | ||
| i += 1 # skip the value |
There was a problem hiding this comment.
What if I specify --batch-type=none (as argparse supports)? Then the next, unrelated, argument gets dropped.
But you shouldn't be parsing the command-line yourself. I suggest you use:
sys.argv = shCommandFromArgs(batchArgs.leftover)
That will strip out all batch-related arguments.
There was a problem hiding this comment.
Much better. I asked around for better solutions than my loop, but none were suggested.
There was a problem hiding this comment.
Good thing I checked; I think that that's
sys.argv = [sys.argv[0]] + batchArgs.leftover
We actually support None and none as all the other batch options are lower case
RobertLuptonTheGood
force-pushed
the
tickets/DM-7900
branch
from
6f22d78
to
db0ccc6
Compare
Not strictly on this ticket, but not worth one of its own
No description provided.