[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-21 Thread ChristopherSchultz
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 Not quite yet. Let's see if we can get it closed automatically through the "proper" channels. --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-21 Thread isapir
Github user isapir commented on the issue: https://github.com/apache/tomcat/pull/56 Should I close it manually? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-21 Thread violetagg
Github user violetagg commented on the issue: https://github.com/apache/tomcat/pull/56 There aren't any replications since yesterday :( --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-21 Thread ChristopherSchultz
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 I'm not sure why the commit message hasn't closed this issue. Any thoughts on why that might be? --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-21 Thread isapir
Github user isapir commented on the issue: https://github.com/apache/tomcat/pull/56 Thank you 👍 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-21 Thread ChristopherSchultz
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 Committed to tomcat-trunk in ASF svn r1799462. Back-ports to follow. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-20 Thread isapir
Github user isapir commented on the issue: https://github.com/apache/tomcat/pull/56 @ChristopherSchultz can we merge this please? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread KeiichiFujino
Github user KeiichiFujino commented on the issue: https://github.com/apache/tomcat/pull/56 I confirmed. there is no problem. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread isapir
Github user isapir commented on the issue: https://github.com/apache/tomcat/pull/56 @KeiichiFujino p.s. This is the last commit that addresses the issues above: https://github.com/apache/tomcat/pull/56/commits/192d2eb13e9ec9448e183dcc3b166f7d3577c250 --- If your project is set up

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread isapir
Github user isapir commented on the issue: https://github.com/apache/tomcat/pull/56 @KeiichiFujino Thank you for your prompt review. >you need to add BackupManager mbean definitions ... in org/apache/catalina/ha/session/mbeans-descriptors.xml Thank you for providing

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread KeiichiFujino
Github user KeiichiFujino commented on the issue: https://github.com/apache/tomcat/pull/56 There are two comments. you need to add BackupManager mbean definitions. ``` ``` in org/apache/catalina/ha/session/mbeans-descriptors.xml There is a typo in

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread isapir
Github user isapir commented on the issue: https://github.com/apache/tomcat/pull/56 @ChristopherSchultz I updated the patch according to your feedback: https://github.com/apache/tomcat/pull/56/commits/f0c3968570f70823d30cf18144e82591a7ee0cee I was actually trying at first to

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread ChristopherSchultz
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 Thanks @Igal for your patience with this PR review. I know we've been a bit of a pain. But this work will make the contribution that much more useful. --- If your project is set up for

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread ChristopherSchultz
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 I'm waiting on a final review from @KeiichiFujino before I commit. In the meantime there are 2 `@return` javadoc annotations with no value. Could you complete those? --- If your

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread isapir
Github user isapir commented on the issue: https://github.com/apache/tomcat/pull/56 I'll change it. Please wait. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread ChristopherSchultz
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 @isapir FYI `1 << bit` is the same as `(int)Math.pow(2, bit)` and probably slightly more efficient. Better yet, it's less code to read. No particular reason to change the patch. I'm

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread isapir
Github user isapir commented on the issue: https://github.com/apache/tomcat/pull/56 @KeiichiFujino @ChristopherSchultz I added: * a method to translate the int value to a string list: https://github.com/apache/tomcat/pull/56/files#diff-5c721c838c78fa7c31f9eb62c27863ceR439

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread isapir
Github user isapir commented on the issue: https://github.com/apache/tomcat/pull/56 >Even if updating the type of channelStartOptions from int to String in mbeans-descriptor, it will display int format values. Because the channelSendOptions is always set to int format. if

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread ChristopherSchultz
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 @KeiichiFujino Okay, so we'd have a synthetic mbean attribute that we can re-construct from the int value we store internally? Or, do you think we should save the string that was used in

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-08 Thread KeiichiFujino
Github user KeiichiFujino commented on the issue: https://github.com/apache/tomcat/pull/56 Sorry for the late reply. I do not think it work as expected. Even if updating the type of channelStartOptions from int to String in mbeans-descriptor, it will display int

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-07 Thread isapir
Github user isapir commented on the issue: https://github.com/apache/tomcat/pull/56 Can we merge this or do we need to wait for @KeiichiFujino 's input? I believe that I addressed all of the issues that he pointed out. --- If your project is set up for it, you can reply to this

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-05 Thread isapir
Github user isapir commented on the issue: https://github.com/apache/tomcat/pull/56 >If you configure channelSendOptions in string form, you should also expose it to JMX in string format. You can see org/apache/catalina/ha/tcp/mbeans-descriptors.xml I updated the type

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-05 Thread isapir
Github user isapir commented on the issue: https://github.com/apache/tomcat/pull/56 >It is necessary to fix configuration docs (webapps/config/cluster.xml). Added >The patch needs to include fix for mapSendOptions. BackupManager use mapSendOptions instead of

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-05 Thread ChristopherSchultz
Github user ChristopherSchultz commented on the issue: https://github.com/apache/tomcat/pull/56 @KeiichiFujino Is this patch acceptable as-is, with additional patches for your items listed above? I think it's okay to add this feature only to channelSendOptions as a first commit.

[GitHub] tomcat issue #56: Convert Cluster Manager human-readable channelSendOptions ...

2017-06-05 Thread KeiichiFujino
Github user KeiichiFujino commented on the issue: https://github.com/apache/tomcat/pull/56 Hi, some comments : - SimpleTcpCluster exposes the configuration via JMX. If you configure channelSendOptions in string form, you should also expose it to JMX in string