----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16130/#review30113 -----------------------------------------------------------
i can already tell that this will be a vast improvement over the status quo. awesome. src/main/python/twitter/aurora/client/cli/__init__.py <https://reviews.apache.org/r/16130/#comment57676> argparse is available in 2.7 but not in 2.6. you should explicitly add a 3rdparty requirement on argparse until we've fully deprecated the use of 2.6 src/main/python/twitter/aurora/client/cli/__init__.py <https://reviews.apache.org/r/16130/#comment57677> if you define multiple exceptions within a class, please create an Error(Exception) base class that they are all derived from. src/main/python/twitter/aurora/client/cli/__init__.py <https://reviews.apache.org/r/16130/#comment57680> if not isinstance(noun, Noun): raise TypeError(...) src/main/python/twitter/aurora/client/cli/__init__.py <https://reviews.apache.org/r/16130/#comment57681> ? src/main/python/twitter/aurora/client/cli/__init__.py <https://reviews.apache.org/r/16130/#comment57682> s/self/cls/ src/main/python/twitter/aurora/client/cli/__init__.py <https://reviews.apache.org/r/16130/#comment57678> this seems like it should be a classmethod. is it only an instancemethod so that @abstractmethod works correctly with it? src/main/python/twitter/aurora/client/cli/__init__.py <https://reviews.apache.org/r/16130/#comment57679> kill src/main/python/twitter/aurora/client/cli/context.py <https://reviews.apache.org/r/16130/#comment57684> sort src/main/python/twitter/aurora/client/cli/jobs.py <https://reviews.apache.org/r/16130/#comment57685> 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 src/main/python/twitter/aurora/client/cli/jobs.py <https://reviews.apache.org/r/16130/#comment57686> any reason not to make parse_shards an argument action? just to keep things simpler? - Brian Wickman On Dec. 10, 2013, 1:26 a.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, 1:26 a.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/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 > >
