Re: Review Request 28350: Add cron replace command.

2014-11-24 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28350/#review62817 --- Discarding this in favor of an upserting cron schedule command.

Review Request 28350: Add cron replace command.

2014-11-21 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28350/ --- Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-921

Re: Review Request 28350: Add cron replace command.

2014-11-21 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28350/#review62676 --- docs/cron-jobs.md

Re: Review Request 28350: Add cron replace command.

2014-11-21 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28350/#review62675 --- docs/cron-jobs.md

Re: Review Request 28350: Add cron replace command.

2014-11-21 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28350/#review62679 --- src/test/python/apache/aurora/client/cli/test_cron.py

Re: Review Request 28350: Add cron replace command.

2014-11-21 Thread Maxim Khutornenko
On Nov. 21, 2014, 11:26 p.m., Kevin Sweeney wrote: docs/cron-jobs.md, line 89 https://reviews.apache.org/r/28350/diff/1/?file=772861#file772861line89 Hold on a sec - why do we have this and cron schedule (see above)? The cron replace is an atomic convenience command for cron

Re: Review Request 28350: Add cron replace command.

2014-11-21 Thread Maxim Khutornenko
On Nov. 21, 2014, 11:34 p.m., David McLaughlin wrote: src/test/python/apache/aurora/client/cli/test_cron.py, lines 199-215 https://reviews.apache.org/r/28350/diff/1/?file=772866#file772866line199 This is an integration test. Can we replace this with a more fine-grained unit test

Re: Review Request 28350: Add cron replace command.

2014-11-21 Thread Maxim Khutornenko
On Nov. 21, 2014, 11:32 p.m., Bill Farner wrote: src/test/python/apache/aurora/client/cli/test_cron.py, lines 218-227 https://reviews.apache.org/r/28350/diff/1/?file=772866#file772866line218 This block is repeated ~verbatim 4x in this file. Cleaning up the underlying code to

Re: Review Request 28350: Add cron replace command.

2014-11-21 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28350/ --- (Updated Nov. 22, 2014, 12:54 a.m.) Review request for Aurora, David

Re: Review Request 28350: Add cron replace command.

2014-11-21 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28350/#review62693 --- Ship it! Ship It! - David McLaughlin On Nov. 22, 2014, 12:54

Re: Review Request 28350: Add cron replace command.

2014-11-21 Thread Kevin Sweeney
On Nov. 21, 2014, 3:26 p.m., Kevin Sweeney wrote: docs/cron-jobs.md, line 89 https://reviews.apache.org/r/28350/diff/1/?file=772861#file772861line89 Hold on a sec - why do we have this and cron schedule (see above)? Maxim Khutornenko wrote: The cron replace is an atomic

Re: Review Request 28350: Add cron replace command.

2014-11-21 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28350/#review62700 --- Ship it! Master (a431b1d) is green with this patch.

Re: Review Request 28350: Add cron replace command.

2014-11-21 Thread Maxim Khutornenko
On Nov. 21, 2014, 11:26 p.m., Kevin Sweeney wrote: docs/cron-jobs.md, line 89 https://reviews.apache.org/r/28350/diff/1/?file=772861#file772861line89 Hold on a sec - why do we have this and cron schedule (see above)? Maxim Khutornenko wrote: The cron replace is an atomic

Re: Review Request 28350: Add cron replace command.

2014-11-21 Thread Maxim Khutornenko
On Nov. 21, 2014, 11:26 p.m., Kevin Sweeney wrote: docs/cron-jobs.md, line 89 https://reviews.apache.org/r/28350/diff/1/?file=772861#file772861line89 Hold on a sec - why do we have this and cron schedule (see above)? Maxim Khutornenko wrote: The cron replace is an atomic