> On Dec. 10, 2013, 2:40 p.m., Brian Wickman wrote: > > src/main/python/twitter/aurora/client/cli/jobs.py, line 92 > > <https://reviews.apache.org/r/16130/diff/2/?file=395807#file395807line92> > > > > any reason not to make parse_shards an argument action? just to keep > > things simpler?
Pure argument actions don't work in argparse. The stuff to replace them is, I think, over-complicated. (It's now done by creating an instance of class Action, and overriding __call__(parser, namespace, values, option). Needing to create a whole nested class just to call parse_shards isn't worth the trouble. > On Dec. 10, 2013, 2:40 p.m., Brian Wickman wrote: > > src/main/python/twitter/aurora/client/cli/jobs.py, lines 81-82 > > <https://reviews.apache.org/r/16130/diff/2/?file=395807#file395807line81> > > > > it seems like the common options should be factored out into an > > options.py kind of like today rather than copy&pasted. > > > > i'd also like to see short options for a few e.g. --open-browser/-o Yeah - I agree. That'll be in the next round of changes to the framework (MESOS-4862). I wanted to first get something working, and see how the code looked. > On Dec. 10, 2013, 2:40 p.m., Brian Wickman wrote: > > src/main/python/twitter/aurora/client/cli/__init__.py, line 146 > > <https://reviews.apache.org/r/16130/diff/2/?file=395805#file395805line146> > > > > this seems like it should be a classmethod. is it only an > > instancemethod so that @abstractmethod works correctly with it? Yes. - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16130/#review30113 ----------------------------------------------------------- On Dec. 10, 2013, 5:14 p.m., Mark Chu-Carroll wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16130/ > ----------------------------------------------------------- > > (Updated Dec. 10, 2013, 5:14 p.m.) > > > Review request for Aurora, Jonathan Boulle and Brian Wickman. > > > Repository: aurora > > > Description > ------- > > First step towards aurora client v2! > > - Initial implementation of the noun/verb command and options processing > framework. > - Initial implementation of a command processing context for aurora commands. > - Implementations of a "Job" noun and "create" and "kill" verbs. > - Tests. > > Note: not all of the v1 tests for the create and kill verbs have been > migrated to the new > framework; command processing contexts need a bit more work to make it easy > to do the > appropriate mocking/stubbing to support them. They'll be in the next change. > > > Diffs > ----- > > src/main/python/twitter/aurora/BUILD.thirdparty > 87156eaa48a39689e18a3190fda487bcfe755771 > src/main/python/twitter/aurora/client/cli/BUILD PRE-CREATION > src/main/python/twitter/aurora/client/cli/__init__.py PRE-CREATION > src/main/python/twitter/aurora/client/cli/context.py PRE-CREATION > src/main/python/twitter/aurora/client/cli/jobs.py PRE-CREATION > src/test/python/twitter/aurora/client/cli/BUILD PRE-CREATION > src/test/python/twitter/aurora/client/cli/test_create.py PRE-CREATION > src/test/python/twitter/aurora/client/cli/test_kill.py PRE-CREATION > src/test/python/twitter/aurora/client/cli/util.py PRE-CREATION > src/test/python/twitter/aurora/client/commands/test_kill.py > 3649969c77f992688a7ddbe592eda8d4edb94036 > > Diff: https://reviews.apache.org/r/16130/diff/ > > > Testing > ------- > > > Thanks, > > Mark Chu-Carroll > >
