You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Udo Kohlmeyer <uk...@gmail.com> on 2016/04/10 23:59:46 UTC
Review Request 45993: GEODE-1201: Adding compileRuntimeLibs to
geode-assembly for tests Amending bind address configuration for
http-service-bind-address
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45993/
-----------------------------------------------------------
Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.
Repository: geode
Description
-------
GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind address configuration for http-service-bind-address
Diffs
-----
geode-assembly/build.gradle b7d05e2ce5fe34d3324e2b55212ff7c8ca02d9e0
geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java a91df05a9931bddd860a79e6b8783fc991ecc2a4
geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java a8921d7d6f2a062a9aa5e27a8376a7e39c8503d5
Diff: https://reviews.apache.org/r/45993/diff/
Testing
-------
battery tests rest.bt
Thanks,
Udo Kohlmeyer
Re: Review Request 45993: GEODE-1201: Adding compileRuntimeLibs to
geode-assembly for tests Amending bind address configuration for
http-service-bind-address
Posted by Udo Kohlmeyer <uk...@gmail.com>.
> On April 11, 2016, 4:44 p.m., Mark Bretl wrote:
> > I agree that we should not be including test jars in our product lib.
> >
> > Also, was precheckin or Travis CI run on this change? It looks like tests not in Geode source were run to verify this change.
The gradle change shall be reverted. Precheckin is in progress.
- Udo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45993/#review128155
-----------------------------------------------------------
On April 10, 2016, 9:59 p.m., Udo Kohlmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45993/
> -----------------------------------------------------------
>
> (Updated April 10, 2016, 9:59 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind address configuration for http-service-bind-address
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle b7d05e2ce5fe34d3324e2b55212ff7c8ca02d9e0
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java a91df05a9931bddd860a79e6b8783fc991ecc2a4
> geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java a8921d7d6f2a062a9aa5e27a8376a7e39c8503d5
>
> Diff: https://reviews.apache.org/r/45993/diff/
>
>
> Testing
> -------
>
> battery tests rest.bt
>
>
> Thanks,
>
> Udo Kohlmeyer
>
>
Re: Review Request 45993: GEODE-1201: Adding compileRuntimeLibs to
geode-assembly for tests Amending bind address configuration for
http-service-bind-address
Posted by Mark Bretl <mb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45993/#review128155
-----------------------------------------------------------
I agree that we should not be including test jars in our product lib.
Also, was precheckin or Travis CI run on this change? It looks like tests not in Geode source were run to verify this change.
- Mark Bretl
On April 10, 2016, 2:59 p.m., Udo Kohlmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45993/
> -----------------------------------------------------------
>
> (Updated April 10, 2016, 2:59 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind address configuration for http-service-bind-address
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle b7d05e2ce5fe34d3324e2b55212ff7c8ca02d9e0
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java a91df05a9931bddd860a79e6b8783fc991ecc2a4
> geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java a8921d7d6f2a062a9aa5e27a8376a7e39c8503d5
>
> Diff: https://reviews.apache.org/r/45993/diff/
>
>
> Testing
> -------
>
> battery tests rest.bt
>
>
> Thanks,
>
> Udo Kohlmeyer
>
>
Re: Review Request 45993: GEODE-1201: Adding compileRuntimeLibs to
geode-assembly for tests Amending bind address configuration for
http-service-bind-address
Posted by Anthony Baker <ab...@apache.org>.
> On April 11, 2016, 3:43 p.m., Bruce Schuchardt wrote:
> > geode-assembly/build.gradle, line 116
> > <https://reviews.apache.org/r/45993/diff/1/?file=1338503#file1338503line116>
> >
> > Is this modifying the assembled product's lib directory?
I agree, doesn't seem right to modify the contents of lib/ to run a unit test.
- Anthony
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45993/#review128137
-----------------------------------------------------------
On April 10, 2016, 9:59 p.m., Udo Kohlmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45993/
> -----------------------------------------------------------
>
> (Updated April 10, 2016, 9:59 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind address configuration for http-service-bind-address
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle b7d05e2ce5fe34d3324e2b55212ff7c8ca02d9e0
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java a91df05a9931bddd860a79e6b8783fc991ecc2a4
> geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java a8921d7d6f2a062a9aa5e27a8376a7e39c8503d5
>
> Diff: https://reviews.apache.org/r/45993/diff/
>
>
> Testing
> -------
>
> battery tests rest.bt
>
>
> Thanks,
>
> Udo Kohlmeyer
>
>
Re: Review Request 45993: GEODE-1201: Adding compileRuntimeLibs to
geode-assembly for tests Amending bind address configuration for
http-service-bind-address
Posted by Udo Kohlmeyer <uk...@gmail.com>.
> On April 11, 2016, 3:43 p.m., Bruce Schuchardt wrote:
> > geode-assembly/build.gradle, line 116
> > <https://reviews.apache.org/r/45993/diff/1/?file=1338503#file1338503line116>
> >
> > Is this modifying the assembled product's lib directory?
>
> Anthony Baker wrote:
> I agree, doesn't seem right to modify the contents of lib/ to run a unit test.
gradle change reverted
- Udo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45993/#review128137
-----------------------------------------------------------
On April 10, 2016, 9:59 p.m., Udo Kohlmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45993/
> -----------------------------------------------------------
>
> (Updated April 10, 2016, 9:59 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind address configuration for http-service-bind-address
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle b7d05e2ce5fe34d3324e2b55212ff7c8ca02d9e0
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java a91df05a9931bddd860a79e6b8783fc991ecc2a4
> geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java a8921d7d6f2a062a9aa5e27a8376a7e39c8503d5
>
> Diff: https://reviews.apache.org/r/45993/diff/
>
>
> Testing
> -------
>
> battery tests rest.bt
>
>
> Thanks,
>
> Udo Kohlmeyer
>
>
Re: Review Request 45993: GEODE-1201: Adding compileRuntimeLibs to
geode-assembly for tests Amending bind address configuration for
http-service-bind-address
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45993/#review128137
-----------------------------------------------------------
geode-assembly/build.gradle (line 116)
<https://reviews.apache.org/r/45993/#comment191507>
Is this modifying the assembled product's lib directory?
- Bruce Schuchardt
On April 10, 2016, 9:59 p.m., Udo Kohlmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45993/
> -----------------------------------------------------------
>
> (Updated April 10, 2016, 9:59 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind address configuration for http-service-bind-address
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle b7d05e2ce5fe34d3324e2b55212ff7c8ca02d9e0
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java a91df05a9931bddd860a79e6b8783fc991ecc2a4
> geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java a8921d7d6f2a062a9aa5e27a8376a7e39c8503d5
>
> Diff: https://reviews.apache.org/r/45993/diff/
>
>
> Testing
> -------
>
> battery tests rest.bt
>
>
> Thanks,
>
> Udo Kohlmeyer
>
>
Re: Review Request 45993: GEODE-1201: Adding compileRuntimeLibs to
geode-assembly for tests Amending bind address configuration for
http-service-bind-address
Posted by Jianxia Chen <jc...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45993/#review128808
-----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java (line 160)
<https://reviews.apache.org/r/45993/#comment192262>
Do you want to add log info of bindAddress as well? e.g. before it returns.
geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java (line 57)
<https://reviews.apache.org/r/45993/#comment192263>
How about using
import org.apache.commons.lang.StringUtils;
?
geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java (line 72)
<https://reviews.apache.org/r/45993/#comment192264>
Do you need to log the bindAddress?
- Jianxia Chen
On April 13, 2016, 7:39 p.m., Udo Kohlmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45993/
> -----------------------------------------------------------
>
> (Updated April 13, 2016, 7:39 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind address configuration for http-service-bind-address
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java a91df05
> geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java a8921d7
>
> Diff: https://reviews.apache.org/r/45993/diff/
>
>
> Testing
> -------
>
> battery tests rest.bt
>
>
> Thanks,
>
> Udo Kohlmeyer
>
>
Re: Review Request 45993: GEODE-1201: Adding compileRuntimeLibs to
geode-assembly for tests Amending bind address configuration for
http-service-bind-address
Posted by Udo Kohlmeyer <uk...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45993/
-----------------------------------------------------------
(Updated April 13, 2016, 7:39 p.m.)
Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.
Changes
-------
reverted gradle change
Repository: geode
Description
-------
GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind address configuration for http-service-bind-address
Diffs (updated)
-----
geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java a91df05
geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java a8921d7
Diff: https://reviews.apache.org/r/45993/diff/
Testing
-------
battery tests rest.bt
Thanks,
Udo Kohlmeyer