Re: Review Request 30389: Deny attempts to create a job update with a non-service job.

2015-02-03 Thread Bill Farner


 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.

2015-02-03 Thread Bill Farner

---
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.

2015-02-03 Thread Maxim Khutornenko


 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.

2015-02-03 Thread Bill Farner

---
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.

2015-02-03 Thread Bill Farner


 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.

2015-02-03 Thread Maxim Khutornenko

---
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.

2015-02-03 Thread Aurora ReviewBot

---
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.

2015-01-29 Thread Aurora ReviewBot

---
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.

2015-01-29 Thread Joshua Cohen

---
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.

2015-01-29 Thread Zameer Manji

---
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.

2015-01-29 Thread Bill Farner


 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.

2015-01-29 Thread Bill Farner


 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.

2015-01-29 Thread Maxim Khutornenko

---
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.

2015-01-29 Thread Bill Farner

---
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.

2015-01-29 Thread Zameer Manji

---
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.

2015-01-29 Thread Bill Farner


 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.

2015-01-29 Thread Joshua Cohen

---
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.

2015-01-29 Thread Aurora ReviewBot

---
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.

2015-01-29 Thread Bill Farner

---
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