You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jinmei Liao <ji...@pivotal.io> on 2016/02/12 00:59:42 UTC
Review Request 43507: GEODE-954: retrieve the hostnameForClients
attribute from the mbean and add a column in the member list view.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43507/
-----------------------------------------------------------
Review request for geode.
Repository: geode
Description
-------
GEODE-954: retrieve the hostnameForClients attribute from the mbean and add a column in the member list view.
Diffs
-----
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/Cluster.java a65a07ef922df83e33e8f785db9dcf1256283747
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java 1cd5fd7b7d976683c09f410e558e0d23c6548cd5
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/PulseConstants.java 9c5732e81e010e73c5a445fc8880b5b8d139e5a9
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/ClusterMemberService.java a3550b542f04862463a75807859ee7a2a57e2147
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MemberDetailsService.java 273ebecdb0bb50b4b4f8cd4509a6b460d7cf7dad
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MembersListService.java f51de1669e510a74fae73294bc105a8280b4e381
gemfire-pulse/src/main/webapp/scripts/pulsescript/clusterDetail.js 8a6c08a80c04816a1b097127b6e459278c7986e3
gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseBaseTest.java 74cfa8dab28249a2491e115f55aeb5cf54f4c6f9
gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTest.java ec0608575ad78953336f2e12b6031d1a71edc81d
Diff: https://reviews.apache.org/r/43507/diff/
Testing
-------
uiTest for pulse
Thanks,
Jinmei Liao
Re: Review Request 43507: GEODE-954: retrieve the hostnameForClients
attribute from the mbean and add a column in the member list view.
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43507/
-----------------------------------------------------------
(Updated Feb. 12, 2016, 9:45 p.m.)
Review request for geode.
Changes
-------
change if/else into switch statements
Repository: geode
Description
-------
GEODE-954: retrieve the hostnameForClients attribute from the mbean and add a column in the member list view.
Diffs (updated)
-----
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/Cluster.java a65a07ef922df83e33e8f785db9dcf1256283747
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java 1cd5fd7b7d976683c09f410e558e0d23c6548cd5
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/PulseConstants.java 9c5732e81e010e73c5a445fc8880b5b8d139e5a9
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/ClusterMemberService.java a3550b542f04862463a75807859ee7a2a57e2147
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MemberDetailsService.java 273ebecdb0bb50b4b4f8cd4509a6b460d7cf7dad
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MembersListService.java f51de1669e510a74fae73294bc105a8280b4e381
gemfire-pulse/src/main/webapp/scripts/pulsescript/clusterDetail.js 8a6c08a80c04816a1b097127b6e459278c7986e3
gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseBaseTest.java 74cfa8dab28249a2491e115f55aeb5cf54f4c6f9
gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTest.java ec0608575ad78953336f2e12b6031d1a71edc81d
Diff: https://reviews.apache.org/r/43507/diff/
Testing
-------
uiTest for pulse
Thanks,
Jinmei Liao
Re: Review Request 43507: GEODE-954: retrieve the hostnameForClients
attribute from the mbean and add a column in the member list view.
Posted by Jinmei Liao <ji...@pivotal.io>.
> On Feb. 12, 2016, 6:27 p.m., Jason Huynh wrote:
> > gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java, line 565
> > <https://reviews.apache.org/r/43507/diff/2/?file=1240866#file1240866line565>
> >
> > Not sure if it matters, but would it be better to pull out memMBean.getKeyProperty(PulseConstants.MBEAN_KEY_PROPERTY_SERVICE) into a local variable and using that in all the else if statements or switch statement?
Yeah, it would be much better. I guess the original code was written pre-java1.7 when switch doesn't work with String? There are lots of code in this file that looks like this. We should do that as part of a larger refactoring effort.
> On Feb. 12, 2016, 6:27 p.m., Jason Huynh wrote:
> > gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java, line 1850
> > <https://reviews.apache.org/r/43507/diff/2/?file=1240866#file1240866line1850>
> >
> > same suggestion here incase attribute.getName() does any type of calculation instead of a simple field get.
Agree.
- Jinmei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43507/#review119054
-----------------------------------------------------------
On Feb. 12, 2016, 6:15 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43507/
> -----------------------------------------------------------
>
> (Updated Feb. 12, 2016, 6:15 p.m.)
>
>
> Review request for geode.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-954: retrieve the hostnameForClients attribute from the mbean and add a column in the member list view.
>
>
> Diffs
> -----
>
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/Cluster.java a65a07ef922df83e33e8f785db9dcf1256283747
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java 1cd5fd7b7d976683c09f410e558e0d23c6548cd5
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/PulseConstants.java 9c5732e81e010e73c5a445fc8880b5b8d139e5a9
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/ClusterMemberService.java a3550b542f04862463a75807859ee7a2a57e2147
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MemberDetailsService.java 273ebecdb0bb50b4b4f8cd4509a6b460d7cf7dad
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MembersListService.java f51de1669e510a74fae73294bc105a8280b4e381
> gemfire-pulse/src/main/webapp/scripts/pulsescript/clusterDetail.js 8a6c08a80c04816a1b097127b6e459278c7986e3
> gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseBaseTest.java 74cfa8dab28249a2491e115f55aeb5cf54f4c6f9
> gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTest.java ec0608575ad78953336f2e12b6031d1a71edc81d
>
> Diff: https://reviews.apache.org/r/43507/diff/
>
>
> Testing
> -------
>
> uiTest for pulse
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 43507: GEODE-954: retrieve the hostnameForClients
attribute from the mbean and add a column in the member list view.
Posted by Jason Huynh <hu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43507/#review119054
-----------------------------------------------------------
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java (line 545)
<https://reviews.apache.org/r/43507/#comment180337>
Not sure if it matters, but would it be better to pull out memMBean.getKeyProperty(PulseConstants.MBEAN_KEY_PROPERTY_SERVICE) into a local variable and using that in all the else if statements or switch statement?
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java (line 1830)
<https://reviews.apache.org/r/43507/#comment180338>
same suggestion here incase attribute.getName() does any type of calculation instead of a simple field get.
- Jason Huynh
On Feb. 12, 2016, 6:15 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43507/
> -----------------------------------------------------------
>
> (Updated Feb. 12, 2016, 6:15 p.m.)
>
>
> Review request for geode.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-954: retrieve the hostnameForClients attribute from the mbean and add a column in the member list view.
>
>
> Diffs
> -----
>
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/Cluster.java a65a07ef922df83e33e8f785db9dcf1256283747
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java 1cd5fd7b7d976683c09f410e558e0d23c6548cd5
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/PulseConstants.java 9c5732e81e010e73c5a445fc8880b5b8d139e5a9
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/ClusterMemberService.java a3550b542f04862463a75807859ee7a2a57e2147
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MemberDetailsService.java 273ebecdb0bb50b4b4f8cd4509a6b460d7cf7dad
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MembersListService.java f51de1669e510a74fae73294bc105a8280b4e381
> gemfire-pulse/src/main/webapp/scripts/pulsescript/clusterDetail.js 8a6c08a80c04816a1b097127b6e459278c7986e3
> gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseBaseTest.java 74cfa8dab28249a2491e115f55aeb5cf54f4c6f9
> gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTest.java ec0608575ad78953336f2e12b6031d1a71edc81d
>
> Diff: https://reviews.apache.org/r/43507/diff/
>
>
> Testing
> -------
>
> uiTest for pulse
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 43507: GEODE-954: retrieve the hostnameForClients
attribute from the mbean and add a column in the member list view.
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43507/
-----------------------------------------------------------
(Updated Feb. 12, 2016, 6:15 p.m.)
Review request for geode.
Changes
-------
change the column header and retrive the bindAddress from the bean as well for display purpose.
Repository: geode
Description
-------
GEODE-954: retrieve the hostnameForClients attribute from the mbean and add a column in the member list view.
Diffs (updated)
-----
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/Cluster.java a65a07ef922df83e33e8f785db9dcf1256283747
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java 1cd5fd7b7d976683c09f410e558e0d23c6548cd5
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/PulseConstants.java 9c5732e81e010e73c5a445fc8880b5b8d139e5a9
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/ClusterMemberService.java a3550b542f04862463a75807859ee7a2a57e2147
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MemberDetailsService.java 273ebecdb0bb50b4b4f8cd4509a6b460d7cf7dad
gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MembersListService.java f51de1669e510a74fae73294bc105a8280b4e381
gemfire-pulse/src/main/webapp/scripts/pulsescript/clusterDetail.js 8a6c08a80c04816a1b097127b6e459278c7986e3
gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseBaseTest.java 74cfa8dab28249a2491e115f55aeb5cf54f4c6f9
gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTest.java ec0608575ad78953336f2e12b6031d1a71edc81d
Diff: https://reviews.apache.org/r/43507/diff/
Testing
-------
uiTest for pulse
Thanks,
Jinmei Liao
Re: Review Request 43507: GEODE-954: retrieve the hostnameForClients
attribute from the mbean and add a column in the member list view.
Posted by Jens Deppe <jd...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43507/#review119010
-----------------------------------------------------------
Should we default hostnameForClients to the regular hostname or bind-address if hostnameForClients isn't set or just leave it blank?
I know this is a bit pedantic, but please use explicit imports rather than import foo.*.
gemfire-pulse/src/main/webapp/scripts/pulsescript/clusterDetail.js (line 767)
<https://reviews.apache.org/r/43507/#comment180310>
Should this not be the fully qualified "Hostname For Clients" string?
- Jens Deppe
On Feb. 11, 2016, 11:59 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43507/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 11:59 p.m.)
>
>
> Review request for geode.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-954: retrieve the hostnameForClients attribute from the mbean and add a column in the member list view.
>
>
> Diffs
> -----
>
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/Cluster.java a65a07ef922df83e33e8f785db9dcf1256283747
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java 1cd5fd7b7d976683c09f410e558e0d23c6548cd5
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/PulseConstants.java 9c5732e81e010e73c5a445fc8880b5b8d139e5a9
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/ClusterMemberService.java a3550b542f04862463a75807859ee7a2a57e2147
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MemberDetailsService.java 273ebecdb0bb50b4b4f8cd4509a6b460d7cf7dad
> gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MembersListService.java f51de1669e510a74fae73294bc105a8280b4e381
> gemfire-pulse/src/main/webapp/scripts/pulsescript/clusterDetail.js 8a6c08a80c04816a1b097127b6e459278c7986e3
> gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseBaseTest.java 74cfa8dab28249a2491e115f55aeb5cf54f4c6f9
> gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTest.java ec0608575ad78953336f2e12b6031d1a71edc81d
>
> Diff: https://reviews.apache.org/r/43507/diff/
>
>
> Testing
> -------
>
> uiTest for pulse
>
>
> Thanks,
>
> Jinmei Liao
>
>