Re: Review Request 35542: GEODE-51 - Hostname for clients property for cache servers

[Kirk Lund]

> On June 24, 2015, 12:35 a.m., Kirk Lund wrote:
> > Ship It!

Issues were dropped because we're opening separate ticket to fix the "help" 
feature in the Locator/ServerLaunchers.


- Kirk


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


On June 20, 2015, 6:10 a.m., William Markito wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35542/
> ---
> 
> (Updated June 20, 2015, 6:10 a.m.)
> 
> 
> Review request for geode, Darrel Schneider and Kirk Lund.
> 
> 
> Bugs: GEODE-51
> https://issues.apache.org/jira/browse/GEODE-51
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> hostname-for-clients property was being ignored by server startup, preventing 
> external clients to connect to the cache servers after connecting to the 
> locator.
> 
> 
> Diffs
> -
> 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java
>  d5db59d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java
>  d5db59d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/i18n/LocalizedStrings.java
>  d5ea8ab 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommands.java
>  68b2483 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/ServerLauncherJUnitTest.java
>  5af33e1 
> 
> Diff: https://reviews.apache.org/r/35542/diff/
> 
> 
> Testing
> ---
> 
> Passed all JUnit tests.
> Manual verification using Docker-composer example.
> JMX MBean values are OK.
> 
> 
> File Attachments
> 
> 
> GEODE-51.patch v2
>   
> https://reviews.apache.org/media/uploaded/files/2015/06/20/01916fb1-9f7d-4847-8a10-59a9ac0824c5__GEODE-51.patch
> 
> 
> Thanks,
> 
> William Markito
> 
>

Re: Review Request 35542: GEODE-51 - Hostname for clients property for cache servers

[Kirk Lund]

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

Ship it!


Ship It!

- Kirk Lund


On June 20, 2015, 6:10 a.m., William Markito wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35542/
> ---
> 
> (Updated June 20, 2015, 6:10 a.m.)
> 
> 
> Review request for geode, Darrel Schneider and Kirk Lund.
> 
> 
> Bugs: GEODE-51
> https://issues.apache.org/jira/browse/GEODE-51
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> hostname-for-clients property was being ignored by server startup, preventing 
> external clients to connect to the cache servers after connecting to the 
> locator.
> 
> 
> Diffs
> -
> 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java
>  d5db59d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java
>  d5db59d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/i18n/LocalizedStrings.java
>  d5ea8ab 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommands.java
>  68b2483 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/ServerLauncherJUnitTest.java
>  5af33e1 
> 
> Diff: https://reviews.apache.org/r/35542/diff/
> 
> 
> Testing
> ---
> 
> Passed all JUnit tests.
> Manual verification using Docker-composer example.
> JMX MBean values are OK.
> 
> 
> File Attachments
> 
> 
> GEODE-51.patch v2
>   
> https://reviews.apache.org/media/uploaded/files/2015/06/20/01916fb1-9f7d-4847-8a10-59a9ac0824c5__GEODE-51.patch
> 
> 
> Thanks,
> 
> William Markito
> 
>

Re: Review Request 35542: GEODE-51 - Hostname for clients property for cache servers

[William Markito]

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

(Updated June 20, 2015, 6:09 a.m.)


Review request for geode, Darrel Schneider and Kirk Lund.


Changes
---

Added help strings


Bugs: GEODE-51
https://issues.apache.org/jira/browse/GEODE-51


Repository: geode


Description
---

hostname-for-clients property was being ignored by server startup, preventing 
external clients to connect to the cache servers after connecting to the 
locator.


Diffs
-

  
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java 
d5db59d 
  
gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommands.java
 68b2483 
  
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/ServerLauncherJUnitTest.java
 5af33e1 

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


Testing
---

Passed all JUnit tests.
Manual verification using Docker-composer example.
JMX MBean values are OK.


File Attachments (updated)


GEODE-51.patch v2
  
https://reviews.apache.org/media/uploaded/files/2015/06/20/01916fb1-9f7d-4847-8a10-59a9ac0824c5__GEODE-51.patch


Thanks,

William Markito

Re: Review Request 35542: GEODE-51 - Hostname for clients property for cache servers

[William Markito]

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

(Updated June 20, 2015, 6:10 a.m.)


Review request for geode, Darrel Schneider and Kirk Lund.


Bugs: GEODE-51
https://issues.apache.org/jira/browse/GEODE-51


Repository: geode


Description
---

hostname-for-clients property was being ignored by server startup, preventing 
external clients to connect to the cache servers after connecting to the 
locator.


Diffs (updated)
-

  
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java 
d5db59d 
  
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java 
d5db59d 
  
gemfire-core/src/main/java/com/gemstone/gemfire/internal/i18n/LocalizedStrings.java
 d5ea8ab 
  
gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommands.java
 68b2483 
  
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/ServerLauncherJUnitTest.java
 5af33e1 

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


Testing
---

Passed all JUnit tests.
Manual verification using Docker-composer example.
JMX MBean values are OK.


File Attachments


GEODE-51.patch v2
  
https://reviews.apache.org/media/uploaded/files/2015/06/20/01916fb1-9f7d-4847-8a10-59a9ac0824c5__GEODE-51.patch


Thanks,

William Markito

Re: Review Request 35542: GEODE-51 - Hostname for clients property for cache servers

[William Markito]

> On June 18, 2015, 5:29 p.m., Darrel Schneider wrote:
> > gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java,
> >  line 1441
> > 
> >
> > I think CliStrings has a bunch more START_SERVER_* constants that are 
> > not listed here. Will those also have this same problem? For example: 
> > START_SERVER__LOAD__POLL__INTERVAL

Can we track this change in another JIRA so we can do a full review of all the 
constants/options that may be missing ? Looks like there are indeed some others 
and I'd like to not only include them, but write proper tests for each one.


- William


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


On June 16, 2015, 11:18 p.m., William Markito wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35542/
> ---
> 
> (Updated June 16, 2015, 11:18 p.m.)
> 
> 
> Review request for geode, Darrel Schneider and Kirk Lund.
> 
> 
> Bugs: GEODE-51
> https://issues.apache.org/jira/browse/GEODE-51
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> hostname-for-clients property was being ignored by server startup, preventing 
> external clients to connect to the cache servers after connecting to the 
> locator.
> 
> 
> Diffs
> -
> 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java
>  d5db59d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommands.java
>  68b2483 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/ServerLauncherJUnitTest.java
>  5af33e1 
> 
> Diff: https://reviews.apache.org/r/35542/diff/
> 
> 
> Testing
> ---
> 
> Passed all JUnit tests.
> Manual verification using Docker-composer example.
> JMX MBean values are OK.
> 
> 
> Thanks,
> 
> William Markito
> 
>

Re: Review Request 35542: GEODE-51 - Hostname for clients property for cache servers

[William Markito]

> On June 18, 2015, 5:51 p.m., Kirk Lund wrote:
> > gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java,
> >  line 1441
> > 
> >
> > Need to add this to helpMap. The other newer arguments that were added 
> > for cluster config also need to be added to helpMap. The LocatorLauncher is 
> > probably similarly effected (newer arguments missing from the helpMap).

Added the helper for the hostname-for-client. But as for the other newer 
arguments I'd say to include as part of the work of another JIRA to review all 
missing constants/options such as the one mentioned by Darrel.


- William


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


On June 16, 2015, 11:18 p.m., William Markito wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35542/
> ---
> 
> (Updated June 16, 2015, 11:18 p.m.)
> 
> 
> Review request for geode, Darrel Schneider and Kirk Lund.
> 
> 
> Bugs: GEODE-51
> https://issues.apache.org/jira/browse/GEODE-51
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> hostname-for-clients property was being ignored by server startup, preventing 
> external clients to connect to the cache servers after connecting to the 
> locator.
> 
> 
> Diffs
> -
> 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java
>  d5db59d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommands.java
>  68b2483 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/ServerLauncherJUnitTest.java
>  5af33e1 
> 
> Diff: https://reviews.apache.org/r/35542/diff/
> 
> 
> Testing
> ---
> 
> Passed all JUnit tests.
> Manual verification using Docker-composer example.
> JMX MBean values are OK.
> 
> 
> Thanks,
> 
> William Markito
> 
>

Re: Review Request 35542: GEODE-51 - Hostname for clients property for cache servers

[Kirk Lund]

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



gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java 
(line 1441)


Need to add this to helpMap. The other newer arguments that were added for 
cluster config also need to be added to helpMap. The LocatorLauncher is 
probably similarly effected (newer arguments missing from the helpMap).


- Kirk Lund


On June 16, 2015, 11:18 p.m., William Markito wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35542/
> ---
> 
> (Updated June 16, 2015, 11:18 p.m.)
> 
> 
> Review request for geode, Darrel Schneider and Kirk Lund.
> 
> 
> Bugs: GEODE-51
> https://issues.apache.org/jira/browse/GEODE-51
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> hostname-for-clients property was being ignored by server startup, preventing 
> external clients to connect to the cache servers after connecting to the 
> locator.
> 
> 
> Diffs
> -
> 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java
>  d5db59d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommands.java
>  68b2483 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/ServerLauncherJUnitTest.java
>  5af33e1 
> 
> Diff: https://reviews.apache.org/r/35542/diff/
> 
> 
> Testing
> ---
> 
> Passed all JUnit tests.
> Manual verification using Docker-composer example.
> JMX MBean values are OK.
> 
> 
> Thanks,
> 
> William Markito
> 
>

Re: Review Request 35542: GEODE-51 - Hostname for clients property for cache servers

[William Markito]

> On June 18, 2015, 5:24 p.m., Darrel Schneider wrote:
> > Why was this option ignored? Seems like it should have given an error about 
> > not recognizing an option.
> 
> William Markito wrote:
> The option was being set but passed along to the ServerLauncher.

*but not passed along


- William


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


On June 16, 2015, 11:18 p.m., William Markito wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35542/
> ---
> 
> (Updated June 16, 2015, 11:18 p.m.)
> 
> 
> Review request for geode, Darrel Schneider and Kirk Lund.
> 
> 
> Bugs: GEODE-51
> https://issues.apache.org/jira/browse/GEODE-51
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> hostname-for-clients property was being ignored by server startup, preventing 
> external clients to connect to the cache servers after connecting to the 
> locator.
> 
> 
> Diffs
> -
> 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java
>  d5db59d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommands.java
>  68b2483 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/ServerLauncherJUnitTest.java
>  5af33e1 
> 
> Diff: https://reviews.apache.org/r/35542/diff/
> 
> 
> Testing
> ---
> 
> Passed all JUnit tests.
> Manual verification using Docker-composer example.
> JMX MBean values are OK.
> 
> 
> Thanks,
> 
> William Markito
> 
>

Re: Review Request 35542: GEODE-51 - Hostname for clients property for cache servers

[William Markito]

> On June 18, 2015, 5:24 p.m., Darrel Schneider wrote:
> > Why was this option ignored? Seems like it should have given an error about 
> > not recognizing an option.

The option was being set but passed along to the ServerLauncher.


- William


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


On June 16, 2015, 11:18 p.m., William Markito wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35542/
> ---
> 
> (Updated June 16, 2015, 11:18 p.m.)
> 
> 
> Review request for geode, Darrel Schneider and Kirk Lund.
> 
> 
> Bugs: GEODE-51
> https://issues.apache.org/jira/browse/GEODE-51
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> hostname-for-clients property was being ignored by server startup, preventing 
> external clients to connect to the cache servers after connecting to the 
> locator.
> 
> 
> Diffs
> -
> 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java
>  d5db59d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommands.java
>  68b2483 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/ServerLauncherJUnitTest.java
>  5af33e1 
> 
> Diff: https://reviews.apache.org/r/35542/diff/
> 
> 
> Testing
> ---
> 
> Passed all JUnit tests.
> Manual verification using Docker-composer example.
> JMX MBean values are OK.
> 
> 
> Thanks,
> 
> William Markito
> 
>

Re: Review Request 35542: GEODE-51 - Hostname for clients property for cache servers

[Darrel Schneider]

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



gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java 
(line 1441)


I think CliStrings has a bunch more START_SERVER_* constants that are not 
listed here. Will those also have this same problem? For example: 
START_SERVER__LOAD__POLL__INTERVAL


- Darrel Schneider


On June 16, 2015, 4:18 p.m., William Markito wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35542/
> ---
> 
> (Updated June 16, 2015, 4:18 p.m.)
> 
> 
> Review request for geode, Darrel Schneider and Kirk Lund.
> 
> 
> Bugs: GEODE-51
> https://issues.apache.org/jira/browse/GEODE-51
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> hostname-for-clients property was being ignored by server startup, preventing 
> external clients to connect to the cache servers after connecting to the 
> locator.
> 
> 
> Diffs
> -
> 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java
>  d5db59d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommands.java
>  68b2483 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/ServerLauncherJUnitTest.java
>  5af33e1 
> 
> Diff: https://reviews.apache.org/r/35542/diff/
> 
> 
> Testing
> ---
> 
> Passed all JUnit tests.
> Manual verification using Docker-composer example.
> JMX MBean values are OK.
> 
> 
> Thanks,
> 
> William Markito
> 
>

Re: Review Request 35542: GEODE-51 - Hostname for clients property for cache servers

[Darrel Schneider]

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


Why was this option ignored? Seems like it should have given an error about not 
recognizing an option.

- Darrel Schneider


On June 16, 2015, 4:18 p.m., William Markito wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35542/
> ---
> 
> (Updated June 16, 2015, 4:18 p.m.)
> 
> 
> Review request for geode, Darrel Schneider and Kirk Lund.
> 
> 
> Bugs: GEODE-51
> https://issues.apache.org/jira/browse/GEODE-51
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> hostname-for-clients property was being ignored by server startup, preventing 
> external clients to connect to the cache servers after connecting to the 
> locator.
> 
> 
> Diffs
> -
> 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java
>  d5db59d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommands.java
>  68b2483 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/ServerLauncherJUnitTest.java
>  5af33e1 
> 
> Diff: https://reviews.apache.org/r/35542/diff/
> 
> 
> Testing
> ---
> 
> Passed all JUnit tests.
> Manual verification using Docker-composer example.
> JMX MBean values are OK.
> 
> 
> Thanks,
> 
> William Markito
> 
>