----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25255/#review52043 -----------------------------------------------------------
Is the command name "supdate" up for debate? I'm not in love with it ;). src/main/python/apache/aurora/client/cli/jobs.py <https://reviews.apache.org/r/25255/#comment90770> This doesn't read very well to me. Maybe something like: "Update action to start an update or pause, resume or abort an in-progress update." src/main/python/apache/aurora/client/cli/jobs.py <https://reviews.apache.org/r/25255/#comment90771> This general form (get cluster api, call api method, check response, report, exit) is repeated for each of the update related methods, the only bits changing are the method to call on the api and the messages that are printed. Consider refactoring to this logic is shared? src/test/python/apache/aurora/client/cli/test_supdate.py <https://reviews.apache.org/r/25255/#comment90773> Similarly to the impl above, there's a lot of commonality between these success test cases that could be refactored to reduce boilerplate. - Joshua Cohen On Sept. 2, 2014, 4:36 p.m., Mark Chu-Carroll wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25255/ > ----------------------------------------------------------- > > (Updated Sept. 2, 2014, 4:36 p.m.) > > > Review request for Aurora. > > > Repository: aurora > > > Description > ------- > > This change contains the basic commands needed to work with the > scheduler-driven updater. (It does not yet cover querying for the status > of the update; that will come in a subsequent change.) > > > Diffs > ----- > > src/main/python/apache/aurora/client/cli/context.py > 51c7d24dca664e476e62f1864d095416dfab70e4 > src/main/python/apache/aurora/client/cli/jobs.py > ebc22aaa5a8aed311897b3ce9632b6f7175b6080 > src/test/python/apache/aurora/client/cli/BUILD > e1f9ebf96774b8f5c75de8570c6ba87d953ab649 > src/test/python/apache/aurora/client/cli/test_restart.py > a1e7a5a94a2d336239df98e2600658b97c546901 > src/test/python/apache/aurora/client/cli/test_supdate.py PRE-CREATION > src/test/python/apache/aurora/client/cli/util.py > 95a2123e127c9811fd2305e71cfc5c7c4376f904 > > Diff: https://reviews.apache.org/r/25255/diff/ > > > Testing > ------- > > New suite of tests for the new command; all unit tests pass. > > > Thanks, > > Mark Chu-Carroll > >
