Review Request 25582: Fix error in client task ssh command when the job isn't found.

2014-09-12 Thread Mark Chu-Carroll

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25582/
---

Review request for Aurora, David McLaughlin and Zameer Manji.


Bugs: aurora-706
https://issues.apache.org/jira/browse/aurora-706


Repository: aurora


Description
---

Fix error in client task ssh command when the job isn't found.


Diffs
-

  src/main/python/apache/aurora/client/cli/task.py 
91175facdc8c9fd59ab66781f86ee8b5940a 
  src/main/python/apache/aurora/client/commands/ssh.py 
37a90089b72b86c82466f1819e7881a36bb2f214 
  src/test/python/apache/aurora/client/cli/test_task_run.py 
8d9ef0543c1ab514d6f039ba63a1d417a4a90a1b 
  src/test/python/apache/aurora/client/commands/test_ssh.py 
4070b710b005c91fe08dd7906cd93bf3a8cdba9e 

Diff: https://reviews.apache.org/r/25582/diff/


Testing
---

Added new tests to catch this case;
Ran all client unit tests, all tests pass.


Thanks,

Mark Chu-Carroll



Re: Review Request 25582: Fix error in client task ssh command when the job isn't found.

2014-09-12 Thread Zameer Manji

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25582/#review53181
---

Ship it!


Ship It!

- Zameer Manji


On Sept. 12, 2014, 7:34 a.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25582/
 ---
 
 (Updated Sept. 12, 2014, 7:34 a.m.)
 
 
 Review request for Aurora, David McLaughlin and Zameer Manji.
 
 
 Bugs: aurora-706
 https://issues.apache.org/jira/browse/aurora-706
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix error in client task ssh command when the job isn't found.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/task.py 
 91175facdc8c9fd59ab66781f86ee8b5940a 
   src/main/python/apache/aurora/client/commands/ssh.py 
 37a90089b72b86c82466f1819e7881a36bb2f214 
   src/test/python/apache/aurora/client/cli/test_task_run.py 
 8d9ef0543c1ab514d6f039ba63a1d417a4a90a1b 
   src/test/python/apache/aurora/client/commands/test_ssh.py 
 4070b710b005c91fe08dd7906cd93bf3a8cdba9e 
 
 Diff: https://reviews.apache.org/r/25582/diff/
 
 
 Testing
 ---
 
 Added new tests to catch this case;
 Ran all client unit tests, all tests pass.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 25582: Fix error in client task ssh command when the job isn't found.

2014-09-12 Thread David McLaughlin

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25582/#review53182
---

Ship it!



src/main/python/apache/aurora/client/cli/task.py
https://reviews.apache.org/r/25582/#comment92643

Is this a debug line?


- David McLaughlin


On Sept. 12, 2014, 2:34 p.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25582/
 ---
 
 (Updated Sept. 12, 2014, 2:34 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Zameer Manji.
 
 
 Bugs: aurora-706
 https://issues.apache.org/jira/browse/aurora-706
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix error in client task ssh command when the job isn't found.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/task.py 
 91175facdc8c9fd59ab66781f86ee8b5940a 
   src/main/python/apache/aurora/client/commands/ssh.py 
 37a90089b72b86c82466f1819e7881a36bb2f214 
   src/test/python/apache/aurora/client/cli/test_task_run.py 
 8d9ef0543c1ab514d6f039ba63a1d417a4a90a1b 
   src/test/python/apache/aurora/client/commands/test_ssh.py 
 4070b710b005c91fe08dd7906cd93bf3a8cdba9e 
 
 Diff: https://reviews.apache.org/r/25582/diff/
 
 
 Testing
 ---
 
 Added new tests to catch this case;
 Ran all client unit tests, all tests pass.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 25582: Fix error in client task ssh command when the job isn't found.

2014-09-12 Thread Joe Smith

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25582/#review53184
---



src/test/python/apache/aurora/client/cli/test_task_run.py
https://reviews.apache.org/r/25582/#comment92646

test_ssh_job_not_found



src/test/python/apache/aurora/client/cli/test_task_run.py
https://reviews.apache.org/r/25582/#comment92647

Test the ssh command for proper behavior when no tasks are found within a 
job or something, I think


- Joe Smith


On Sept. 12, 2014, 7:34 a.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25582/
 ---
 
 (Updated Sept. 12, 2014, 7:34 a.m.)
 
 
 Review request for Aurora, David McLaughlin and Zameer Manji.
 
 
 Bugs: aurora-706
 https://issues.apache.org/jira/browse/aurora-706
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix error in client task ssh command when the job isn't found.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/task.py 
 91175facdc8c9fd59ab66781f86ee8b5940a 
   src/main/python/apache/aurora/client/commands/ssh.py 
 37a90089b72b86c82466f1819e7881a36bb2f214 
   src/test/python/apache/aurora/client/cli/test_task_run.py 
 8d9ef0543c1ab514d6f039ba63a1d417a4a90a1b 
   src/test/python/apache/aurora/client/commands/test_ssh.py 
 4070b710b005c91fe08dd7906cd93bf3a8cdba9e 
 
 Diff: https://reviews.apache.org/r/25582/diff/
 
 
 Testing
 ---
 
 Added new tests to catch this case;
 Ran all client unit tests, all tests pass.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 25582: Fix error in client task ssh command when the job isn't found.

2014-09-12 Thread Joshua Cohen


 On Sept. 12, 2014, 5:09 p.m., Joe Smith wrote:
  src/test/python/apache/aurora/client/cli/test_task_run.py, line 228
  https://reviews.apache.org/r/25582/diff/1/?file=687672#file687672line228
 
  Test the ssh command for proper behavior when no tasks are found 
  within a job or something, I think

I'd go so far as to suggest that docstrings on test methods are probably not 
necessary. This exemplifies why, they just get copied/pasted from other tests 
and end up not accurately describing what each test does. I'd vote for 
descriptive test case names and do away with docstrings entirely.


- Joshua


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25582/#review53184
---


On Sept. 12, 2014, 2:34 p.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25582/
 ---
 
 (Updated Sept. 12, 2014, 2:34 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Zameer Manji.
 
 
 Bugs: aurora-706
 https://issues.apache.org/jira/browse/aurora-706
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix error in client task ssh command when the job isn't found.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/task.py 
 91175facdc8c9fd59ab66781f86ee8b5940a 
   src/main/python/apache/aurora/client/commands/ssh.py 
 37a90089b72b86c82466f1819e7881a36bb2f214 
   src/test/python/apache/aurora/client/cli/test_task_run.py 
 8d9ef0543c1ab514d6f039ba63a1d417a4a90a1b 
   src/test/python/apache/aurora/client/commands/test_ssh.py 
 4070b710b005c91fe08dd7906cd93bf3a8cdba9e 
 
 Diff: https://reviews.apache.org/r/25582/diff/
 
 
 Testing
 ---
 
 Added new tests to catch this case;
 Ran all client unit tests, all tests pass.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 25582: Fix error in client task ssh command when the job isn't found.

2014-09-12 Thread Mark Chu-Carroll

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25582/
---

(Updated Sept. 12, 2014, 1:17 p.m.)


Review request for Aurora, David McLaughlin and Zameer Manji.


Changes
---

Address review comments.


Bugs: aurora-706
https://issues.apache.org/jira/browse/aurora-706


Repository: aurora


Description
---

Fix error in client task ssh command when the job isn't found.


Diffs (updated)
-

  src/main/python/apache/aurora/client/cli/task.py 
91175facdc8c9fd59ab66781f86ee8b5940a 
  src/main/python/apache/aurora/client/commands/ssh.py 
37a90089b72b86c82466f1819e7881a36bb2f214 
  src/test/python/apache/aurora/client/cli/test_task_run.py 
8d9ef0543c1ab514d6f039ba63a1d417a4a90a1b 
  src/test/python/apache/aurora/client/commands/test_ssh.py 
4070b710b005c91fe08dd7906cd93bf3a8cdba9e 

Diff: https://reviews.apache.org/r/25582/diff/


Testing
---

Added new tests to catch this case;
Ran all client unit tests, all tests pass.


Thanks,

Mark Chu-Carroll



Re: Review Request 25582: Fix error in client task ssh command when the job isn't found.

2014-09-12 Thread Joe Smith

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25582/#review53189
---

Ship it!


Ship It!

- Joe Smith


On Sept. 12, 2014, 10:17 a.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25582/
 ---
 
 (Updated Sept. 12, 2014, 10:17 a.m.)
 
 
 Review request for Aurora, David McLaughlin and Zameer Manji.
 
 
 Bugs: aurora-706
 https://issues.apache.org/jira/browse/aurora-706
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix error in client task ssh command when the job isn't found.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/task.py 
 91175facdc8c9fd59ab66781f86ee8b5940a 
   src/main/python/apache/aurora/client/commands/ssh.py 
 37a90089b72b86c82466f1819e7881a36bb2f214 
   src/test/python/apache/aurora/client/cli/test_task_run.py 
 8d9ef0543c1ab514d6f039ba63a1d417a4a90a1b 
   src/test/python/apache/aurora/client/commands/test_ssh.py 
 4070b710b005c91fe08dd7906cd93bf3a8cdba9e 
 
 Diff: https://reviews.apache.org/r/25582/diff/
 
 
 Testing
 ---
 
 Added new tests to catch this case;
 Ran all client unit tests, all tests pass.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 25582: Fix error in client task ssh command when the job isn't found.

2014-09-12 Thread Joe Smith


 On Sept. 12, 2014, 10:09 a.m., Joe Smith wrote:
  src/test/python/apache/aurora/client/cli/test_task_run.py, line 228
  https://reviews.apache.org/r/25582/diff/1/?file=687672#file687672line228
 
  Test the ssh command for proper behavior when no tasks are found 
  within a job or something, I think
 
 Joshua Cohen wrote:
 I'd go so far as to suggest that docstrings on test methods are probably 
 not necessary. This exemplifies why, they just get copied/pasted from other 
 tests and end up not accurately describing what each test does. I'd vote for 
 descriptive test case names and do away with docstrings entirely.
 
 David McLaughlin wrote:
 +1

Yeah, that's probably the right move overall, though I'm okay with docstrings 
for test methods if they give a bit more clarification as opposed to increasing 
an already-long test-method name.


- Joe


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25582/#review53184
---


On Sept. 12, 2014, 10:17 a.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25582/
 ---
 
 (Updated Sept. 12, 2014, 10:17 a.m.)
 
 
 Review request for Aurora, David McLaughlin and Zameer Manji.
 
 
 Bugs: aurora-706
 https://issues.apache.org/jira/browse/aurora-706
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix error in client task ssh command when the job isn't found.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/task.py 
 91175facdc8c9fd59ab66781f86ee8b5940a 
   src/main/python/apache/aurora/client/commands/ssh.py 
 37a90089b72b86c82466f1819e7881a36bb2f214 
   src/test/python/apache/aurora/client/cli/test_task_run.py 
 8d9ef0543c1ab514d6f039ba63a1d417a4a90a1b 
   src/test/python/apache/aurora/client/commands/test_ssh.py 
 4070b710b005c91fe08dd7906cd93bf3a8cdba9e 
 
 Diff: https://reviews.apache.org/r/25582/diff/
 
 
 Testing
 ---
 
 Added new tests to catch this case;
 Ran all client unit tests, all tests pass.
 
 
 Thanks,
 
 Mark Chu-Carroll