Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-20 Thread Stephan Erb 

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


Ship it!




Ship It!

- Stephan Erb


On Sept. 19, 2016, 8:28 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51980/
> ---
> 
> (Updated Sept. 19, 2016, 8:28 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1772
> https://issues.apache.org/jira/browse/AURORA-1772
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor with addition of HttpClient injected into Webhook class as 
> opposed to previous usage of lower level HtttpURLConnection objects. 
> Additionally due to peformance issues, it is unncessary to POST entire task 
> state to webhook endpoint on every scheduler restart so that is removed in 
> this patch.
> 
> 
> Diffs
> -
> 
>   build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> e54aa19d67278fcb5586f07c399f09062f845a18 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> eaa70533a64740c74cebf15739b7142e2d3a4d11 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json 
> 00787985510d7d415b8074bef06d28b635c78b09 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> 488eefd14c3e67a41a75c809397c8d19f83cc08a 
> 
> Diff: https://reviews.apache.org/r/51980/diff/
> 
> 
> Testing
> ---
> 
> Part of reason for refactor is to allow easier testing so cleaner unit tests 
> were added with more code coverage.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-20 Thread Maxim Khutornenko 

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


Ship it!




Ship It!

- Maxim Khutornenko


On Sept. 19, 2016, 6:28 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51980/
> ---
> 
> (Updated Sept. 19, 2016, 6:28 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1772
> https://issues.apache.org/jira/browse/AURORA-1772
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor with addition of HttpClient injected into Webhook class as 
> opposed to previous usage of lower level HtttpURLConnection objects. 
> Additionally due to peformance issues, it is unncessary to POST entire task 
> state to webhook endpoint on every scheduler restart so that is removed in 
> this patch.
> 
> 
> Diffs
> -
> 
>   build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> e54aa19d67278fcb5586f07c399f09062f845a18 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> eaa70533a64740c74cebf15739b7142e2d3a4d11 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json 
> 00787985510d7d415b8074bef06d28b635c78b09 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> 488eefd14c3e67a41a75c809397c8d19f83cc08a 
> 
> Diff: https://reviews.apache.org/r/51980/diff/
> 
> 
> Testing
> ---
> 
> Part of reason for refactor is to allow easier testing so cleaner unit tests 
> were added with more code coverage.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-19 Thread Zameer Manji 

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


Ship it!




Ship It!

- Zameer Manji


On Sept. 19, 2016, 11:28 a.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51980/
> ---
> 
> (Updated Sept. 19, 2016, 11:28 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1772
> https://issues.apache.org/jira/browse/AURORA-1772
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor with addition of HttpClient injected into Webhook class as 
> opposed to previous usage of lower level HtttpURLConnection objects. 
> Additionally due to peformance issues, it is unncessary to POST entire task 
> state to webhook endpoint on every scheduler restart so that is removed in 
> this patch.
> 
> 
> Diffs
> -
> 
>   build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> e54aa19d67278fcb5586f07c399f09062f845a18 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> eaa70533a64740c74cebf15739b7142e2d3a4d11 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json 
> 00787985510d7d415b8074bef06d28b635c78b09 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> 488eefd14c3e67a41a75c809397c8d19f83cc08a 
> 
> Diff: https://reviews.apache.org/r/51980/diff/
> 
> 
> Testing
> ---
> 
> Part of reason for refactor is to allow easier testing so cleaner unit tests 
> were added with more code coverage.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-19 Thread Aurora ReviewBot 

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


Ship it!




Master (4c4040f) 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 Sept. 19, 2016, 6:28 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51980/
> ---
> 
> (Updated Sept. 19, 2016, 6:28 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1772
> https://issues.apache.org/jira/browse/AURORA-1772
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor with addition of HttpClient injected into Webhook class as 
> opposed to previous usage of lower level HtttpURLConnection objects. 
> Additionally due to peformance issues, it is unncessary to POST entire task 
> state to webhook endpoint on every scheduler restart so that is removed in 
> this patch.
> 
> 
> Diffs
> -
> 
>   build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> e54aa19d67278fcb5586f07c399f09062f845a18 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> eaa70533a64740c74cebf15739b7142e2d3a4d11 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json 
> 00787985510d7d415b8074bef06d28b635c78b09 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> 488eefd14c3e67a41a75c809397c8d19f83cc08a 
> 
> Diff: https://reviews.apache.org/r/51980/diff/
> 
> 
> Testing
> ---
> 
> Part of reason for refactor is to allow easier testing so cleaner unit tests 
> were added with more code coverage.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-19 Thread Dmitriy Shirchenko 

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



@ReviewBot retry

- Dmitriy Shirchenko


On Sept. 19, 2016, 6:28 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51980/
> ---
> 
> (Updated Sept. 19, 2016, 6:28 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1772
> https://issues.apache.org/jira/browse/AURORA-1772
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor with addition of HttpClient injected into Webhook class as 
> opposed to previous usage of lower level HtttpURLConnection objects. 
> Additionally due to peformance issues, it is unncessary to POST entire task 
> state to webhook endpoint on every scheduler restart so that is removed in 
> this patch.
> 
> 
> Diffs
> -
> 
>   build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> e54aa19d67278fcb5586f07c399f09062f845a18 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> eaa70533a64740c74cebf15739b7142e2d3a4d11 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json 
> 00787985510d7d415b8074bef06d28b635c78b09 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> 488eefd14c3e67a41a75c809397c8d19f83cc08a 
> 
> Diff: https://reviews.apache.org/r/51980/diff/
> 
> 
> Testing
> ---
> 
> Part of reason for refactor is to allow easier testing so cleaner unit tests 
> were added with more code coverage.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-19 Thread Aurora ReviewBot 

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



Master (330e8ee) is red with this patch.
  ./build-support/jenkins/build.sh

 # Create file stdout for capturing output. We 
can't use StringIO mock
 # because TestProcess is running fork.
 with open(os.path.join(td, 'sys_stdout'), 
'w+') as stdout:
   with open(os.path.join(td, 'sys_stderr'), 
'w+') as stderr:
 with mutable_sys():
   sys.stdout, sys.stderr = stdout, 
stderr
 
   p = TestProcess('process', 'echo hello 
world; echo >&2 hello stderr', 0,
   taskpath, sandbox, 
logger_destination=LoggerDestination.BOTH)
   p.start()
   rc = 
wait_for_rc(taskpath.getpath('process_checkpoint'))
 
   assert rc == 0
   # Check log files were created in std 
path with correct content
 > assert_log_content(taskpath, 'stdout', 
'hello world\n')
 
 src/test/python/apache/thermos/core/test_process.py:487: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 taskpath = 
 log_name = 'stdout'
 expected_content = 'hello world\n'
 
 def assert_log_content(taskpath, log_name, 
expected_content):
   log = 
taskpath.with_filename(log_name).getpath('process_logdir')
   assert os.path.exists(log)
   with open(log, 'r') as fp:
 >   assert fp.read() == expected_content
 E   assert '' == 'hello world\n'
 E + hello world
 
 src/test/python/apache/thermos/core/test_process.py:313: 
AssertionError
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  1 failed, 710 passed, 6 skipped, 1 warnings in 
327.06 seconds 
 
FAILURE


19:09:25 06:10   [complete]
   FAILURE


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Sept. 19, 2016, 6:28 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51980/
> ---
> 
> (Updated Sept. 19, 2016, 6:28 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1772
> https://issues.apache.org/jira/browse/AURORA-1772
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor with addition of HttpClient injected into Webhook class as 
> opposed to previous usage of lower level HtttpURLConnection objects. 
> Additionally due to peformance issues, it is unncessary to POST entire task 
> state to webhook endpoint on every scheduler restart so that is removed in 
> this patch.
> 
> 
> Diffs
> -
> 
>   build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> e54aa19d67278fcb5586f07c399f09062f845a18 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> eaa70533a64740c74cebf15739b7142e2d3a4d11 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json 
> 00787985510d7d415b8074bef06d28b635c78b09 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> 488eefd14c3e67a41a75c809397c8d19f83cc08a 
> 
> Diff: https://reviews.apache.org/r/51980/diff/
> 
> 
> Testing
> ---
> 
> Part of reason for refactor is to allow easier testing so cleaner unit tests 
> were added with more code coverage.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-19 Thread Dmitriy Shirchenko 

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

(Updated Sept. 19, 2016, 6:28 p.m.)


Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.


Bugs: AURORA-1772
https://issues.apache.org/jira/browse/AURORA-1772


Repository: aurora


Description
---

This is a refactor with addition of HttpClient injected into Webhook class as 
opposed to previous usage of lower level HtttpURLConnection objects. 
Additionally due to peformance issues, it is unncessary to POST entire task 
state to webhook endpoint on every scheduler restart so that is removed in this 
patch.


Diffs (updated)
-

  build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
  src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
e54aa19d67278fcb5586f07c399f09062f845a18 
  src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
  src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
eaa70533a64740c74cebf15739b7142e2d3a4d11 
  src/main/resources/org/apache/aurora/scheduler/webhook.json 
00787985510d7d415b8074bef06d28b635c78b09 
  src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
488eefd14c3e67a41a75c809397c8d19f83cc08a 

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


Testing
---

Part of reason for refactor is to allow easier testing so cleaner unit tests 
were added with more code coverage.


Thanks,

Dmitriy Shirchenko

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-19 Thread Zameer Manji 

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




src/main/java/org/apache/aurora/scheduler/events/Webhook.java (line 71)


Please leave a comment here that if the old state is not present it's 
because the scheduler just started up and we don't want to resend those event .


- Zameer Manji


On Sept. 16, 2016, 6:40 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51980/
> ---
> 
> (Updated Sept. 16, 2016, 6:40 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1772
> https://issues.apache.org/jira/browse/AURORA-1772
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor with addition of HttpClient injected into Webhook class as 
> opposed to previous usage of lower level HtttpURLConnection objects. 
> Additionally due to peformance issues, it is unncessary to POST entire task 
> state to webhook endpoint on every scheduler restart so that is removed in 
> this patch.
> 
> 
> Diffs
> -
> 
>   build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> e54aa19d67278fcb5586f07c399f09062f845a18 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> eaa70533a64740c74cebf15739b7142e2d3a4d11 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json 
> 00787985510d7d415b8074bef06d28b635c78b09 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> 488eefd14c3e67a41a75c809397c8d19f83cc08a 
> 
> Diff: https://reviews.apache.org/r/51980/diff/
> 
> 
> Testing
> ---
> 
> Part of reason for refactor is to allow easier testing so cleaner unit tests 
> were added with more code coverage.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-16 Thread Dmitriy Shirchenko 

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

(Updated Sept. 17, 2016, 1:40 a.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Bugs: AURORA-1772
https://issues.apache.org/jira/browse/AURORA-1772


Repository: aurora


Description (updated)
---

This is a refactor with addition of HttpClient injected into Webhook class as 
opposed to previous usage of lower level HtttpURLConnection objects. 
Additionally due to peformance issues, it is unncessary to POST entire task 
state to webhook endpoint on every scheduler restart so that is removed in this 
patch.


Diffs
-

  build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
  src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
e54aa19d67278fcb5586f07c399f09062f845a18 
  src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
  src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
eaa70533a64740c74cebf15739b7142e2d3a4d11 
  src/main/resources/org/apache/aurora/scheduler/webhook.json 
00787985510d7d415b8074bef06d28b635c78b09 
  src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
488eefd14c3e67a41a75c809397c8d19f83cc08a 

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


Testing
---

Part of reason for refactor is to allow easier testing so cleaner unit tests 
were added with more code coverage.


Thanks,

Dmitriy Shirchenko

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-16 Thread Aurora ReviewBot 

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


Ship it!




Master (a87ad41) 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 Sept. 17, 2016, 12:48 a.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51980/
> ---
> 
> (Updated Sept. 17, 2016, 12:48 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1772
> https://issues.apache.org/jira/browse/AURORA-1772
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor with addition of HttpClient to use more lower level 
> HtttpURLConnection objects to inject the client into Webhook class. 
> Additionally due to peformance issues, it is unncessary to POST entire task 
> state to webhook endpoint on every scheduler restart.
> 
> 
> Diffs
> -
> 
>   build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> e54aa19d67278fcb5586f07c399f09062f845a18 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> eaa70533a64740c74cebf15739b7142e2d3a4d11 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json 
> 00787985510d7d415b8074bef06d28b635c78b09 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> 488eefd14c3e67a41a75c809397c8d19f83cc08a 
> 
> Diff: https://reviews.apache.org/r/51980/diff/
> 
> 
> Testing
> ---
> 
> Part of reason for refactor is to allow easier testing so cleaner unit tests 
> were added with more code coverage.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>