You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by joewitt <gi...@git.apache.org> on 2016/07/22 02:44:38 UTC
[GitHub] nifi pull request #704: NIFI-2253 flexibly cleaning zookeeper connect string
GitHub user joewitt opened a pull request:
https://github.com/apache/nifi/pull/704
NIFI-2253 flexibly cleaning zookeeper connect string
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/joewitt/incubator-nifi NIFI-2253
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/704.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #704
----
commit 891d404bf95b7654bf1f1cb5c7c40a1efcb6b7fe
Author: joewitt <jo...@apache.org>
Date: 2016-07-22T02:43:15Z
NIFI-2253 flexibly cleaning zookeeper connect string
----
---
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 the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #704: NIFI-2253 flexibly cleaning zookeeper connect string
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/704
---
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 the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #704: NIFI-2253 flexibly cleaning zookeeper connect string
Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/704#discussion_r71884405
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/cluster/ZooKeeperClientConfig.java ---
@@ -70,7 +75,10 @@ public static ZooKeeperClientConfig createConfig(final Properties properties) {
if (connectString == null || connectString.trim().isEmpty()) {
throw new IllegalStateException("The '" + NiFiProperties.ZOOKEEPER_CONNECT_STRING + "' property is not set in nifi.properties");
}
-
+ final String cleanedConnectString = cleanConnectString(connectString);
+ if (cleanedConnectString.isEmpty()) {
+ throw new IllegalStateException("The '" + NiFiProperties.ZOOKEEPER_CONNECT_STRING + "' property is set but contains unrecognized data in nifi.properties");
--- End diff --
This message could probably be made a little more clear - what is "unrecognized data"? Perhaps spell out that the expected format is a comma-separated list of <hostname>:<port> pairs.
---
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 the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #704: NIFI-2253 flexibly cleaning zookeeper connect string
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/704
+1
Visually verified code, did a contrib check build. In 3 node secure cluster, I added spaces to various parts of each nifi.properties and then ran the cluster. Performed as expected. Thanks @joewitt, I will merge it in.
---
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 the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #704: NIFI-2253 flexibly cleaning zookeeper connect string
Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/704#discussion_r71884169
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/cluster/ZooKeeperClientConfig.java ---
@@ -93,4 +101,42 @@ private static int getTimePeriod(final Properties properties, final String prope
return (int) FormatUtils.getTimeDuration(defaultValue, TimeUnit.MILLISECONDS);
}
}
+
+ /**
+ * Takes a given connect string and splits it by ',' character. For each
+ * split result trims whitespace then splits by ':' character. For each
+ * secondary split if a single value is returned it is trimmed and then the
+ * default zookeeper 2181 is append by adding ":2181". If two values are
+ * returned then the second value is evaluated to ensure it contains only
+ * digits and if not then the entry is skipped. If more than two values are
+ * returned the entry is skipped. Each entry is trimmed and if empty the
+ * entry is skipped. After all splits are cleaned then they are all appended
+ * back together demarcated by "," and the full string is returned.
+ *
+ * @param connectString the string to clean
+ * @return cleaned connect string guaranteed to be non null but could be
+ * empty
+ */
+ public static String cleanConnectString(final String connectString) {
+ final String nospaces = StringUtils.deleteWhitespace(connectString);
+ final String hostPortPairs[] = StringUtils.split(nospaces, ",", 100);
+ final List<String> cleanedEntries = new ArrayList<>(hostPortPairs.length);
+ for (final String pair : hostPortPairs) {
+ final String pairSplits[] = StringUtils.split(pair, ":", 3);
+ if (pairSplits.length > 2) {
+ continue; //skip it
--- End diff --
In these cases, where we have received an invalid configuration, I would be much more inclined to throw an Exception than to simply ignore the value. At a minimum, I think we need to log a warning message indicating that we are skipping the value that the user provided us.
---
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 the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---