Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
On Feb. 3, 2015, 8:20 p.m., Bill Farner wrote: Maxim - ping? Maxim Khutornenko wrote: my previous comment is not addressed yet? Ah sorry, i missed that. Will reply and/or update the diff. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70818 --- On Jan. 29, 2015, 9:26 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 9:26 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_inspect.py ae04776fc32b2f09ce6303aeb893c60963896d60 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70818 --- Maxim - ping? - Bill Farner On Jan. 29, 2015, 9:26 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 9:26 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_inspect.py ae04776fc32b2f09ce6303aeb893c60963896d60 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
On Feb. 3, 2015, 8:20 p.m., Bill Farner wrote: Maxim - ping? my previous comment is not addressed yet? - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70818 --- On Jan. 29, 2015, 9:26 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 9:26 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_inspect.py ae04776fc32b2f09ce6303aeb893c60963896d60 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Feb. 3, 2015, 11:53 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_inspect.py ae04776fc32b2f09ce6303aeb893c60963896d60 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
On Jan. 29, 2015, 7:21 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/jobs.py, lines 738-741 https://reviews.apache.org/r/30389/diff/1/?file=839595#file839595line738 This is techincally not true for the client updater. Cron jobs are supported despite the fact that we now have cron commands to do so. I suggest narrowing down this validation to ad-hoc jobs only. Good catch, modified this check. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70252 --- On Feb. 3, 2015, 11:53 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Feb. 3, 2015, 11:53 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_inspect.py ae04776fc32b2f09ce6303aeb893c60963896d60 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70865 --- Ship it! Ship It! - Maxim Khutornenko On Feb. 3, 2015, 11:53 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Feb. 3, 2015, 11:53 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_inspect.py ae04776fc32b2f09ce6303aeb893c60963896d60 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70874 --- Ship it! Master (2742120) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Feb. 3, 2015, 11:53 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Feb. 3, 2015, 11:53 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_inspect.py ae04776fc32b2f09ce6303aeb893c60963896d60 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70233 --- Ship it! Master (4f04a34) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Jan. 29, 2015, 5:13 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 5:13 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70244 --- src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/30389/#comment115321 Should we include the proper way to update a non-service job (kill/create) in this message? - Joshua Cohen On Jan. 29, 2015, 5:13 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 5:13 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70248 --- src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java https://reviews.apache.org/r/30389/#comment115326 Why is this change necessary? src/test/python/apache/aurora/client/cli/test_update.py https://reviews.apache.org/r/30389/#comment115328 There has been some previous discussion that this style of test brittle and we should avoid it. Would you mind moving this test up to the TestJobUpdateCommand suite at the top of this file? This test could then be modified to have minimal patching. - Zameer Manji On Jan. 29, 2015, 9:13 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 9:13 a.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
On Jan. 29, 2015, 6:49 p.m., Joshua Cohen wrote: src/main/python/apache/aurora/client/cli/jobs.py, line 741 https://reviews.apache.org/r/30389/diff/1/?file=839595#file839595line741 Should we include the proper way to update a non-service job (kill/create) in this message? Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70244 --- On Jan. 29, 2015, 5:13 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 5:13 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
On Jan. 29, 2015, 7:15 p.m., Zameer Manji wrote: src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java, line 3116 https://reviews.apache.org/r/30389/diff/1/?file=839596#file839596line3116 Why is this change necessary? Thrift-generated code assumes collections are mutable, specifically in the `addTo*` methods. This is used in `ConfigurationManager`. On Jan. 29, 2015, 7:15 p.m., Zameer Manji wrote: src/test/python/apache/aurora/client/cli/test_update.py, line 464 https://reviews.apache.org/r/30389/diff/1/?file=839597#file839597line464 There has been some previous discussion that this style of test brittle and we should avoid it. Would you mind moving this test up to the TestJobUpdateCommand suite at the top of this file? This test could then be modified to have minimal patching. I had no idea there was a bifurcation of test approach in this file...wow. Moved. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70248 --- On Jan. 29, 2015, 5:13 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 5:13 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70252 --- src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/30389/#comment115331 This is techincally not true for the client updater. Cron jobs are supported despite the fact that we now have cron commands to do so. I suggest narrowing down this validation to ad-hoc jobs only. - Maxim Khutornenko On Jan. 29, 2015, 5:13 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 5:13 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 7:31 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_inspect.py ae04776fc32b2f09ce6303aeb893c60963896d60 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70255 --- Ship it! src/test/python/apache/aurora/client/cli/test_update.py https://reviews.apache.org/r/30389/#comment115334 This seems like a code smell. Shouldn't we be able to configure the fake_context to return the job_config instead of setting the method to a mock? This doesn't block my shipping of this patch but I would appreciate some investigation into this pattern. - Zameer Manji On Jan. 29, 2015, 11:31 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 11:31 a.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_inspect.py ae04776fc32b2f09ce6303aeb893c60963896d60 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
On Jan. 29, 2015, 7:36 p.m., Zameer Manji wrote: src/test/python/apache/aurora/client/cli/test_update.py, line 99 https://reviews.apache.org/r/30389/diff/1-2/?file=839597#file839597line99 This seems like a code smell. Shouldn't we be able to configure the fake_context to return the job_config instead of setting the method to a mock? This doesn't block my shipping of this patch but I would appreciate some investigation into this pattern. I choose to punt. This points out a more widespread issue with the overuse of `AuroraCommandContext`. I propose we aim to break down that class and turn it into a container class. Then, classes like this can inject mocks rather than constructing mocks for the purposes of wiring in other mocks. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70255 --- On Jan. 29, 2015, 7:31 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 7:31 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_inspect.py ae04776fc32b2f09ce6303aeb893c60963896d60 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70274 --- Ship it! Ship It! - Joshua Cohen On Jan. 29, 2015, 7:31 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 7:31 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_inspect.py ae04776fc32b2f09ce6303aeb893c60963896d60 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/#review70269 --- Master (4f04a34) is red with this patch. ./build-support/jenkins/build.sh SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/util.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/__init__.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/test_host_maintenance.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/test_admin_util.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/test_maintenance.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/test_admin.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/test_admin_sla.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/admin/util.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_transport.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_cluster_option.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_cluster.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_clusters.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_pex_version.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_shellify.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_aurora_job_key.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/common/test_http_signaler.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/config/test_constraint_parsing.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/config/test_thrift.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/config/test_loader.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/config/test_base.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_status_manager.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_executor_vars.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_executor_base.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_thermos_executor.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_gc_executor.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_thermos_task_runner.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/test_executor_detector.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_announcer.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_kill_manager.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/__init__.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_executor_timeout.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/fixtures.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/executor/common/test_health_checker.py Everything Looks Good! SUCCESS:
Re: Review Request 30389: Deny attempts to create a job update with a non-service job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30389/ --- (Updated Jan. 29, 2015, 9:26 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-330 https://issues.apache.org/jira/browse/AURORA-330 Repository: aurora Description --- Deny attempts to create a job update with a non-service job. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8c19f3b08135eb5f3098591ebf9931b42a086318 src/main/python/apache/aurora/client/cli/jobs.py 7c5374417f8cca7400c7e92d014f706c0b2368fd src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 src/test/python/apache/aurora/client/cli/test_inspect.py ae04776fc32b2f09ce6303aeb893c60963896d60 src/test/python/apache/aurora/client/cli/test_update.py c470ee64f11b5a1e4ce8cf1635c1acd2ec6e6e40 src/test/python/apache/aurora/client/cli/util.py 5b6207d45eba9ecc24cfd6dc5910677f9bc44372 Diff: https://reviews.apache.org/r/30389/diff/ Testing --- Thanks, Bill Farner