Re: Review Request 35587: Suppress task reconciliation status update logging.

2015-06-22 Thread Bill Farner 

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

Ship it!


Ship It!

- Bill Farner


On June 19, 2015, 10:07 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35587/
> ---
> 
> (Updated June 19, 2015, 10:07 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Suppress task reconciliation status update logging.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> f233d5a181bb1f43fbbfe657dbda2cf975aa6895 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> f0f9ac392973a276028aee3e06517a6e6d960bb6 
> 
> Diff: https://reviews.apache.org/r/35587/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 35587: Suppress task reconciliation status update logging.

2015-06-19 Thread Aurora ReviewBot 

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

Ship it!


Master (d240926) 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 June 19, 2015, 10:07 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35587/
> ---
> 
> (Updated June 19, 2015, 10:07 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Suppress task reconciliation status update logging.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> f233d5a181bb1f43fbbfe657dbda2cf975aa6895 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> f0f9ac392973a276028aee3e06517a6e6d960bb6 
> 
> Diff: https://reviews.apache.org/r/35587/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 35587: Suppress task reconciliation status update logging.

2015-06-19 Thread Maxim Khutornenko 


> On June 18, 2015, 4:42 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java,
> >  line 314
> > 
> >
> > What's being tested here?  If i'm reading correctly, this test case 
> > would pass before the patch.
> 
> Maxim Khutornenko wrote:
> This is mostly for test coverage similarly to the 
> testOfferFirstAcceptsFineLogging above. Asserting a message would require 
> replacing current anonymous logger with a mock and deeper refactoring. Do you 
> think it's worth it here?
> 
> Bill Farner wrote:
> You could pass a mock/stub `Logger` and expect the behavior (if you do, 
> please be vague with expectations to avoid coupling to log message strings, 
> etc).  If that's not to your liking, you could punt.

Done.


- Maxim


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


On June 19, 2015, 10:07 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35587/
> ---
> 
> (Updated June 19, 2015, 10:07 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Suppress task reconciliation status update logging.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> f233d5a181bb1f43fbbfe657dbda2cf975aa6895 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> f0f9ac392973a276028aee3e06517a6e6d960bb6 
> 
> Diff: https://reviews.apache.org/r/35587/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 35587: Suppress task reconciliation status update logging.

2015-06-19 Thread Maxim Khutornenko 

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

(Updated June 19, 2015, 10:07 p.m.)


Review request for Aurora and Bill Farner.


Changes
---

Bill's comments.


Repository: aurora


Description
---

Suppress task reconciliation status update logging.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
f233d5a181bb1f43fbbfe657dbda2cf975aa6895 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
f0f9ac392973a276028aee3e06517a6e6d960bb6 

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


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 35587: Suppress task reconciliation status update logging.

2015-06-18 Thread Aurora ReviewBot 

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

Ship it!


Master (af7e1a7) 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 June 18, 2015, 5:04 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35587/
> ---
> 
> (Updated June 18, 2015, 5:04 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Suppress task reconciliation status update logging.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> f233d5a181bb1f43fbbfe657dbda2cf975aa6895 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> f0f9ac392973a276028aee3e06517a6e6d960bb6 
> 
> Diff: https://reviews.apache.org/r/35587/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 35587: Suppress task reconciliation status update logging.

2015-06-18 Thread Bill Farner 


> On June 18, 2015, 4:42 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java,
> >  line 314
> > 
> >
> > What's being tested here?  If i'm reading correctly, this test case 
> > would pass before the patch.
> 
> Maxim Khutornenko wrote:
> This is mostly for test coverage similarly to the 
> testOfferFirstAcceptsFineLogging above. Asserting a message would require 
> replacing current anonymous logger with a mock and deeper refactoring. Do you 
> think it's worth it here?

You could pass a mock/stub `Logger` and expect the behavior (if you do, please 
be vague with expectations to avoid coupling to log message strings, etc).  If 
that's not to your liking, you could punt.


- Bill


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


On June 18, 2015, 5:04 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35587/
> ---
> 
> (Updated June 18, 2015, 5:04 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Suppress task reconciliation status update logging.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> f233d5a181bb1f43fbbfe657dbda2cf975aa6895 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> f0f9ac392973a276028aee3e06517a6e6d960bb6 
> 
> Diff: https://reviews.apache.org/r/35587/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 35587: Suppress task reconciliation status update logging.

2015-06-18 Thread Maxim Khutornenko 

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

(Updated June 18, 2015, 5:04 p.m.)


Review request for Aurora and Bill Farner.


Changes
---

Bill's comments.


Repository: aurora


Description
---

Suppress task reconciliation status update logging.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
f233d5a181bb1f43fbbfe657dbda2cf975aa6895 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
f0f9ac392973a276028aee3e06517a6e6d960bb6 

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


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 35587: Suppress task reconciliation status update logging.

2015-06-18 Thread Maxim Khutornenko 


> On June 18, 2015, 4:42 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java,
> >  line 314
> > 
> >
> > What's being tested here?  If i'm reading correctly, this test case 
> > would pass before the patch.

This is mostly for test coverage similarly to the 
testOfferFirstAcceptsFineLogging above. Asserting a message would require 
replacing current anonymous logger with a mock and deeper refactoring. Do you 
think it's worth it here?


> On June 18, 2015, 4:42 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, 
> > line 198
> > 
> >
> > A bit more context will help future readers.  You could mention that 
> > reconciliation is expected to result in many no-op status updates.

Sure, done.


- Maxim


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


On June 18, 2015, 1:02 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35587/
> ---
> 
> (Updated June 18, 2015, 1:02 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Suppress task reconciliation status update logging.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> f233d5a181bb1f43fbbfe657dbda2cf975aa6895 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> f0f9ac392973a276028aee3e06517a6e6d960bb6 
> 
> Diff: https://reviews.apache.org/r/35587/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 35587: Suppress task reconciliation status update logging.

2015-06-18 Thread Bill Farner 

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



src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java (line 
198)


A bit more context will help future readers.  You could mention that 
reconciliation is expected to result in many no-op status updates.



src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
(line 314)


What's being tested here?  If i'm reading correctly, this test case would 
pass before the patch.


- Bill Farner


On June 18, 2015, 1:02 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35587/
> ---
> 
> (Updated June 18, 2015, 1:02 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Suppress task reconciliation status update logging.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> f233d5a181bb1f43fbbfe657dbda2cf975aa6895 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> f0f9ac392973a276028aee3e06517a6e6d960bb6 
> 
> Diff: https://reviews.apache.org/r/35587/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 35587: Suppress task reconciliation status update logging.

2015-06-17 Thread Aurora ReviewBot 

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

Ship it!


Master (af7e1a7) 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 June 18, 2015, 1:02 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35587/
> ---
> 
> (Updated June 18, 2015, 1:02 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Suppress task reconciliation status update logging.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> f233d5a181bb1f43fbbfe657dbda2cf975aa6895 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> f0f9ac392973a276028aee3e06517a6e6d960bb6 
> 
> Diff: https://reviews.apache.org/r/35587/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Review Request 35587: Suppress task reconciliation status update logging.

2015-06-17 Thread Maxim Khutornenko 

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

Review request for Aurora and Bill Farner.


Repository: aurora


Description
---

Suppress task reconciliation status update logging.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
f233d5a181bb1f43fbbfe657dbda2cf975aa6895 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
f0f9ac392973a276028aee3e06517a6e6d960bb6 

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


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko