You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/02/25 00:07:05 UTC
[GitHub] [zookeeper] ctest-team opened a new pull request #1266:
ZOOKEEPER-3721: Making the boolean configuration parameters consistent
ctest-team opened a new pull request #1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
URL: https://github.com/apache/zookeeper/pull/1266
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] ctest-team commented on issue #1266: ZOOKEEPER-3721:
Making the boolean configuration parameters consistent
Posted by GitBox <gi...@apache.org>.
ctest-team commented on issue #1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
URL: https://github.com/apache/zookeeper/pull/1266#issuecomment-592714919
@maoling
Is there anything else we need to commit this one?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] marshtompsxd commented on a change in pull request
#1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
Posted by GitBox <gi...@apache.org>.
marshtompsxd commented on a change in pull request #1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
URL: https://github.com/apache/zookeeper/pull/1266#discussion_r384272768
##########
File path: zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java
##########
@@ -164,6 +164,31 @@ public void testJvmPauseMonitorConfigured() throws IOException, ConfigException
assertTrue(quorumPeerConfig.isJvmPauseMonitorToRun());
}
+ /**
+ * Test case for https://issues.apache.org/jira/browse/ZOOKEEPER-3721
+ */
+ @Test
+ public void testParseBoolean() throws IOException, ConfigException {
+ QuorumPeerConfig quorumPeerConfig = new QuorumPeerConfig();
+ Properties zkProp = getDefaultZKProperties();
+
+ zkProp.setProperty("localSessionsEnabled", "true");
+ quorumPeerConfig.parseProperties(zkProp);
+ assertEquals(true, quorumPeerConfig.areLocalSessionsEnabled());
+
+ zkProp.setProperty("localSessionsEnabled", "false");
+ quorumPeerConfig.parseProperties(zkProp);
+ assertEquals(false, quorumPeerConfig.areLocalSessionsEnabled());
+
+ zkProp.setProperty("localSessionsEnabled", "yes");
+ try {
+ quorumPeerConfig.parseProperties(zkProp);
+ fail("Must throw exception as 'yes' is not accpetable for parseBoolean!");
Review comment:
Do you mean things like:
```
zkProp.setProperty("localSessionsEnabled", "True");
quorumPeerConfig.parseProperties(zkProp);
assertEquals(true, quorumPeerConfig.areLocalSessionsEnabled());
zkProp.setProperty("localSessionsEnabled", "False");
quorumPeerConfig.parseProperties(zkProp);
assertEquals(false, quorumPeerConfig.areLocalSessionsEnabled());
```
to test whether ignoring case can work?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] ctest-team edited a comment on issue #1266:
ZOOKEEPER-3721: Making the boolean configuration parameters consistent
Posted by GitBox <gi...@apache.org>.
ctest-team edited a comment on issue #1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
URL: https://github.com/apache/zookeeper/pull/1266#issuecomment-590961039
@maoling OK. I will write a test and push again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] marshtompsxd commented on a change in pull request
#1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
Posted by GitBox <gi...@apache.org>.
marshtompsxd commented on a change in pull request #1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
URL: https://github.com/apache/zookeeper/pull/1266#discussion_r384272768
##########
File path: zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java
##########
@@ -164,6 +164,31 @@ public void testJvmPauseMonitorConfigured() throws IOException, ConfigException
assertTrue(quorumPeerConfig.isJvmPauseMonitorToRun());
}
+ /**
+ * Test case for https://issues.apache.org/jira/browse/ZOOKEEPER-3721
+ */
+ @Test
+ public void testParseBoolean() throws IOException, ConfigException {
+ QuorumPeerConfig quorumPeerConfig = new QuorumPeerConfig();
+ Properties zkProp = getDefaultZKProperties();
+
+ zkProp.setProperty("localSessionsEnabled", "true");
+ quorumPeerConfig.parseProperties(zkProp);
+ assertEquals(true, quorumPeerConfig.areLocalSessionsEnabled());
+
+ zkProp.setProperty("localSessionsEnabled", "false");
+ quorumPeerConfig.parseProperties(zkProp);
+ assertEquals(false, quorumPeerConfig.areLocalSessionsEnabled());
+
+ zkProp.setProperty("localSessionsEnabled", "yes");
+ try {
+ quorumPeerConfig.parseProperties(zkProp);
+ fail("Must throw exception as 'yes' is not accpetable for parseBoolean!");
Review comment:
Do you mean things like:
```
zkProp.setProperty("localSessionsEnabled", "True");
quorumPeerConfig.parseProperties(zkProp);
assertEquals(true, quorumPeerConfig.areLocalSessionsEnabled());
zkProp.setProperty("localSessionsEnabled", "False");
quorumPeerConfig.parseProperties(zkProp);
assertEquals(false, quorumPeerConfig.areLocalSessionsEnabled());
```
to test whether ignoring case can work?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] ctest-team removed a comment on issue #1266:
ZOOKEEPER-3721: Making the boolean configuration parameters consistent
Posted by GitBox <gi...@apache.org>.
ctest-team removed a comment on issue #1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
URL: https://github.com/apache/zookeeper/pull/1266#issuecomment-590960780
@maoling OK. I will write a test and pr again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] ctest-team commented on issue #1266: ZOOKEEPER-3721:
Making the boolean configuration parameters consistent
Posted by GitBox <gi...@apache.org>.
ctest-team commented on issue #1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
URL: https://github.com/apache/zookeeper/pull/1266#issuecomment-590961039
@maoling OK. I will write a test and push again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] maoling commented on a change in pull request #1266:
ZOOKEEPER-3721: Making the boolean configuration parameters consistent
Posted by GitBox <gi...@apache.org>.
maoling commented on a change in pull request #1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
URL: https://github.com/apache/zookeeper/pull/1266#discussion_r384304641
##########
File path: zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java
##########
@@ -164,6 +164,31 @@ public void testJvmPauseMonitorConfigured() throws IOException, ConfigException
assertTrue(quorumPeerConfig.isJvmPauseMonitorToRun());
}
+ /**
+ * Test case for https://issues.apache.org/jira/browse/ZOOKEEPER-3721
+ */
+ @Test
+ public void testParseBoolean() throws IOException, ConfigException {
+ QuorumPeerConfig quorumPeerConfig = new QuorumPeerConfig();
+ Properties zkProp = getDefaultZKProperties();
+
+ zkProp.setProperty("localSessionsEnabled", "true");
+ quorumPeerConfig.parseProperties(zkProp);
+ assertEquals(true, quorumPeerConfig.areLocalSessionsEnabled());
+
+ zkProp.setProperty("localSessionsEnabled", "false");
+ quorumPeerConfig.parseProperties(zkProp);
+ assertEquals(false, quorumPeerConfig.areLocalSessionsEnabled());
+
+ zkProp.setProperty("localSessionsEnabled", "yes");
+ try {
+ quorumPeerConfig.parseProperties(zkProp);
+ fail("Must throw exception as 'yes' is not accpetable for parseBoolean!");
Review comment:
Yes it is
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] ctest-team commented on a change in pull request #1266:
ZOOKEEPER-3721: Making the boolean configuration parameters consistent
Posted by GitBox <gi...@apache.org>.
ctest-team commented on a change in pull request #1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
URL: https://github.com/apache/zookeeper/pull/1266#discussion_r384273838
##########
File path: zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java
##########
@@ -164,6 +164,31 @@ public void testJvmPauseMonitorConfigured() throws IOException, ConfigException
assertTrue(quorumPeerConfig.isJvmPauseMonitorToRun());
}
+ /**
+ * Test case for https://issues.apache.org/jira/browse/ZOOKEEPER-3721
+ */
+ @Test
+ public void testParseBoolean() throws IOException, ConfigException {
+ QuorumPeerConfig quorumPeerConfig = new QuorumPeerConfig();
+ Properties zkProp = getDefaultZKProperties();
+
+ zkProp.setProperty("localSessionsEnabled", "true");
+ quorumPeerConfig.parseProperties(zkProp);
+ assertEquals(true, quorumPeerConfig.areLocalSessionsEnabled());
+
+ zkProp.setProperty("localSessionsEnabled", "false");
+ quorumPeerConfig.parseProperties(zkProp);
+ assertEquals(false, quorumPeerConfig.areLocalSessionsEnabled());
+
+ zkProp.setProperty("localSessionsEnabled", "yes");
+ try {
+ quorumPeerConfig.parseProperties(zkProp);
+ fail("Must throw exception as 'yes' is not accpetable for parseBoolean!");
Review comment:
Do you mean things like:
```
zkProp.setProperty("localSessionsEnabled", "True");
quorumPeerConfig.parseProperties(zkProp);
assertEquals(true, quorumPeerConfig.areLocalSessionsEnabled());
zkProp.setProperty("localSessionsEnabled", "False");
quorumPeerConfig.parseProperties(zkProp);
assertEquals(false, quorumPeerConfig.areLocalSessionsEnabled());
```
to test whether ignoring case can work?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] ctest-team commented on issue #1266: ZOOKEEPER-3721:
Making the boolean configuration parameters consistent
Posted by GitBox <gi...@apache.org>.
ctest-team commented on issue #1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
URL: https://github.com/apache/zookeeper/pull/1266#issuecomment-591625276
@maoling Thank you for the suggestion.
I pushed again to add the test for `True` and `False`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] maoling commented on a change in pull request #1266:
ZOOKEEPER-3721: Making the boolean configuration parameters consistent
Posted by GitBox <gi...@apache.org>.
maoling commented on a change in pull request #1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
URL: https://github.com/apache/zookeeper/pull/1266#discussion_r384249424
##########
File path: zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java
##########
@@ -164,6 +164,31 @@ public void testJvmPauseMonitorConfigured() throws IOException, ConfigException
assertTrue(quorumPeerConfig.isJvmPauseMonitorToRun());
}
+ /**
+ * Test case for https://issues.apache.org/jira/browse/ZOOKEEPER-3721
+ */
+ @Test
+ public void testParseBoolean() throws IOException, ConfigException {
+ QuorumPeerConfig quorumPeerConfig = new QuorumPeerConfig();
+ Properties zkProp = getDefaultZKProperties();
+
+ zkProp.setProperty("localSessionsEnabled", "true");
+ quorumPeerConfig.parseProperties(zkProp);
+ assertEquals(true, quorumPeerConfig.areLocalSessionsEnabled());
+
+ zkProp.setProperty("localSessionsEnabled", "false");
+ quorumPeerConfig.parseProperties(zkProp);
+ assertEquals(false, quorumPeerConfig.areLocalSessionsEnabled());
+
+ zkProp.setProperty("localSessionsEnabled", "yes");
+ try {
+ quorumPeerConfig.parseProperties(zkProp);
+ fail("Must throw exception as 'yes' is not accpetable for parseBoolean!");
Review comment:
we also need to assert `True` or `False` ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] ctest-team commented on issue #1266: ZOOKEEPER-3721:
Making the boolean configuration parameters consistent
Posted by GitBox <gi...@apache.org>.
ctest-team commented on issue #1266: ZOOKEEPER-3721: Making the boolean configuration parameters consistent
URL: https://github.com/apache/zookeeper/pull/1266#issuecomment-590978240
@maoling
Thank you and I just pushed the test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services