You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2014/10/30 07:02:06 UTC
Review Request 27375: Remove dependency on application-http.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27375/
-----------------------------------------------------------
Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
Repository: aurora
Description
-------
We were only using this for DefaultQuitHandler, which is easy enough to replace.
Additionally, common:io is only used in tests.
Diffs
-----
build.gradle 719ceb75aa462a7e3d0f06602facdd866f18ee5d
src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 958f60c9d804af4c915a53ccdaf489a91ee284c7
src/main/java/org/apache/aurora/scheduler/http/QuitCallback.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/QuitCallbackTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/27375/diff/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 27375: Remove dependency on application-http.
Posted by Bill Farner <wf...@apache.org>.
> On Oct. 30, 2014, 9:49 a.m., Joshua Cohen wrote:
> > src/test/java/org/apache/aurora/scheduler/http/QuitCallbackTest.java, lines 38-43
> > <https://reviews.apache.org/r/27375/diff/1/?file=742747#file742747line38>
> >
> > Maybe nitpicky, but should the behavior under test here be that Lifecycle#shutdown is invoked, rather than that Lifecycle will execute the command when shutdown is called? I.e., instead of setting up a QuitCallback with a real Lifecycle that's given a mock Command, just set it up with a mock Lifecycle and assert that shutdown is called? The former relies on knowledge of what Lifecycle does on shutdown which seems out of scope for this test.
That was actually my initial approach, but Lifecycle is a class and shutdown is final, disqualifying it for EasyMock. As we progress down this path, we will translate Lifecycle into an interface.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27375/#review59162
-----------------------------------------------------------
On Oct. 30, 2014, 6:02 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27375/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 6:02 a.m.)
>
>
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> We were only using this for DefaultQuitHandler, which is easy enough to replace.
>
> Additionally, common:io is only used in tests.
>
>
> Diffs
> -----
>
> build.gradle 719ceb75aa462a7e3d0f06602facdd866f18ee5d
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 958f60c9d804af4c915a53ccdaf489a91ee284c7
> src/main/java/org/apache/aurora/scheduler/http/QuitCallback.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/QuitCallbackTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27375/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 27375: Remove dependency on application-http.
Posted by Joshua Cohen <jc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27375/#review59162
-----------------------------------------------------------
Ship it!
One down...
src/test/java/org/apache/aurora/scheduler/http/QuitCallbackTest.java
<https://reviews.apache.org/r/27375/#comment100471>
Maybe nitpicky, but should the behavior under test here be that Lifecycle#shutdown is invoked, rather than that Lifecycle will execute the command when shutdown is called? I.e., instead of setting up a QuitCallback with a real Lifecycle that's given a mock Command, just set it up with a mock Lifecycle and assert that shutdown is called? The former relies on knowledge of what Lifecycle does on shutdown which seems out of scope for this test.
- Joshua Cohen
On Oct. 30, 2014, 6:02 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27375/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 6:02 a.m.)
>
>
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> We were only using this for DefaultQuitHandler, which is easy enough to replace.
>
> Additionally, common:io is only used in tests.
>
>
> Diffs
> -----
>
> build.gradle 719ceb75aa462a7e3d0f06602facdd866f18ee5d
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 958f60c9d804af4c915a53ccdaf489a91ee284c7
> src/main/java/org/apache/aurora/scheduler/http/QuitCallback.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/QuitCallbackTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27375/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 27375: Remove dependency on application-http.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27375/#review59207
-----------------------------------------------------------
Ship it!
Ship It!
- Maxim Khutornenko
On Oct. 30, 2014, 6:02 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27375/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 6:02 a.m.)
>
>
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> We were only using this for DefaultQuitHandler, which is easy enough to replace.
>
> Additionally, common:io is only used in tests.
>
>
> Diffs
> -----
>
> build.gradle 719ceb75aa462a7e3d0f06602facdd866f18ee5d
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 958f60c9d804af4c915a53ccdaf489a91ee284c7
> src/main/java/org/apache/aurora/scheduler/http/QuitCallback.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/QuitCallbackTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27375/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 27375: Remove dependency on application-http.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27375/#review59143
-----------------------------------------------------------
Ship it!
Master (d85e616) is green with this patch.
./build-support/jenkins/build.sh
- Aurora ReviewBot
On Oct. 30, 2014, 6:02 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27375/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 6:02 a.m.)
>
>
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> We were only using this for DefaultQuitHandler, which is easy enough to replace.
>
> Additionally, common:io is only used in tests.
>
>
> Diffs
> -----
>
> build.gradle 719ceb75aa462a7e3d0f06602facdd866f18ee5d
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 958f60c9d804af4c915a53ccdaf489a91ee284c7
> src/main/java/org/apache/aurora/scheduler/http/QuitCallback.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/QuitCallbackTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27375/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 27375: Remove dependency on application-http.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27375/#review59420
-----------------------------------------------------------
Ship it!
Master (765f2dd) is green with this patch.
./build-support/jenkins/build.sh
- Aurora ReviewBot
On Oct. 31, 2014, 9:03 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27375/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2014, 9:03 p.m.)
>
>
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> We were only using this for DefaultQuitHandler, which is easy enough to replace.
>
> Additionally, common:io is only used in tests.
>
>
> Diffs
> -----
>
> build.gradle 719ceb75aa462a7e3d0f06602facdd866f18ee5d
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 958f60c9d804af4c915a53ccdaf489a91ee284c7
> src/main/java/org/apache/aurora/scheduler/http/QuitCallback.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/QuitCallbackTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27375/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 27375: Remove dependency on application-http.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27375/
-----------------------------------------------------------
(Updated Oct. 31, 2014, 9:03 p.m.)
Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
Repository: aurora
Description
-------
We were only using this for DefaultQuitHandler, which is easy enough to replace.
Additionally, common:io is only used in tests.
Diffs (updated)
-----
build.gradle 719ceb75aa462a7e3d0f06602facdd866f18ee5d
src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 958f60c9d804af4c915a53ccdaf489a91ee284c7
src/main/java/org/apache/aurora/scheduler/http/QuitCallback.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/QuitCallbackTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/27375/diff/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 27375: Remove dependency on application-http.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27375/
-----------------------------------------------------------
(Updated Oct. 31, 2014, 9:02 p.m.)
Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
Repository: aurora
Description
-------
We were only using this for DefaultQuitHandler, which is easy enough to replace.
Additionally, common:io is only used in tests.
Diffs (updated)
-----
build.gradle 719ceb75aa462a7e3d0f06602facdd866f18ee5d
src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 958f60c9d804af4c915a53ccdaf489a91ee284c7
src/main/java/org/apache/aurora/scheduler/http/QuitCallback.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/QuitCallbackTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/27375/diff/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 27375: Remove dependency on application-http.
Posted by Bill Farner <wf...@apache.org>.
> On Oct. 30, 2014, 6:15 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/http/QuitCallback.java, line 37
> > <https://reviews.apache.org/r/27375/diff/1/?file=742746#file742746line37>
> >
> > to shutdown?
Done.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27375/#review59237
-----------------------------------------------------------
On Oct. 30, 2014, 6:02 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27375/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 6:02 a.m.)
>
>
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> We were only using this for DefaultQuitHandler, which is easy enough to replace.
>
> Additionally, common:io is only used in tests.
>
>
> Diffs
> -----
>
> build.gradle 719ceb75aa462a7e3d0f06602facdd866f18ee5d
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 958f60c9d804af4c915a53ccdaf489a91ee284c7
> src/main/java/org/apache/aurora/scheduler/http/QuitCallback.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/QuitCallbackTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27375/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 27375: Remove dependency on application-http.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27375/#review59237
-----------------------------------------------------------
Ship it!
Ship It!
src/main/java/org/apache/aurora/scheduler/http/QuitCallback.java
<https://reviews.apache.org/r/27375/#comment100514>
to shutdown?
- Kevin Sweeney
On Oct. 29, 2014, 11:02 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27375/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2014, 11:02 p.m.)
>
>
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> We were only using this for DefaultQuitHandler, which is easy enough to replace.
>
> Additionally, common:io is only used in tests.
>
>
> Diffs
> -----
>
> build.gradle 719ceb75aa462a7e3d0f06602facdd866f18ee5d
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 958f60c9d804af4c915a53ccdaf489a91ee284c7
> src/main/java/org/apache/aurora/scheduler/http/QuitCallback.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/QuitCallbackTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27375/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>