You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by ecararus <gi...@git.apache.org> on 2017/07/19 15:34:06 UTC
[GitHub] storm pull request #2228: fix class cast exception when nimbus seeds is not ...
GitHub user ecararus opened a pull request:
https://github.com/apache/storm/pull/2228
fix class cast exception when nimbus seeds is not an array of hosts j…
Can't use ["host"] it is : - > Caused by: java.lang.ClassCastException: java.lang.String cannot be cast to java.util.List at org.apache.storm.utils.NimbusClient.getConfiguredClientAs(NimbusClient.java:76)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ecararus/storm bugfix/fix_class_cast_exception
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2228.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 #2228
----
----
---
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] storm issue #2228: STORM-2646 NimbusClient Class cast exception when nimbus ...
Posted by ecararus <gi...@git.apache.org>.
Github user ecararus commented on the issue:
https://github.com/apache/storm/pull/2228
@srdo my topology.yaml has:
```
config:
nimbus.seeds: ${nimbus.seeds}
```
The property from dev.properties is loaded by [FluxParser](https://github.com/apache/storm/blob/master/flux/flux-core/src/main/java/org/apache/storm/flux/parser/FluxParser.java#L86) see line 100 there ```nimbus.seeds``` is treated as string and when on NimbusClient.java we try to :
```
seeds = (List<String>) conf.get(Config.NIMBUS_SEEDS);
```
then I'm getting class cast ex. and in the exception we can see mesage Can't use ["host"] it is ....
As @revans2 suggested before cast we need to check if this is an array, in the situation described above it is configured as array of hosts but parser istreating as a string with "[host]" .
---
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] storm pull request #2228: STORM-2646 NimbusClient Class cast exception when ...
Posted by ecararus <gi...@git.apache.org>.
Github user ecararus commented on a diff in the pull request:
https://github.com/apache/storm/pull/2228#discussion_r128377962
--- Diff: storm-client/src/jvm/org/apache/storm/utils/NimbusClient.java ---
@@ -109,12 +109,13 @@ public static NimbusClient getConfiguredClientAs(Map<String, Object> conf, Strin
}
List<String> seeds;
- if(conf.containsKey(Config.NIMBUS_HOST)) {
+ if (conf.containsKey(Config.NIMBUS_HOST)) {
LOG.warn("Using deprecated config {} for backward compatibility. Please update your storm.yaml so it only has config {}",
- Config.NIMBUS_HOST, Config.NIMBUS_SEEDS);
+ Config.NIMBUS_HOST, Config.NIMBUS_SEEDS);
seeds = Lists.newArrayList(conf.get(Config.NIMBUS_HOST).toString());
} else {
- seeds = (List<String>) conf.get(Config.NIMBUS_SEEDS);
+ Object nimbusSeeds = conf.get(Config.NIMBUS_SEEDS);
+ seeds = nimbusSeeds instanceof String[] ? Lists.newArrayList((String[]) nimbusSeeds) : Lists.newArrayList(nimbusSeeds.toString());
--- End diff --
@d2r I don't think that ```conf.get(Config.NIMBUS_SEEDS)``` will return List<String> as per contract the values in conf is Object. in order to avoid any issue probably is better to have a config validation as @revans2 suggested.
---
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] storm pull request #2228: fix class cast exception when nimbus seeds is not ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2228#discussion_r128316430
--- Diff: storm-client/src/jvm/org/apache/storm/utils/NimbusClient.java ---
@@ -109,12 +109,13 @@ public static NimbusClient getConfiguredClientAs(Map<String, Object> conf, Strin
}
List<String> seeds;
- if(conf.containsKey(Config.NIMBUS_HOST)) {
+ if (conf.containsKey(Config.NIMBUS_HOST)) {
LOG.warn("Using deprecated config {} for backward compatibility. Please update your storm.yaml so it only has config {}",
- Config.NIMBUS_HOST, Config.NIMBUS_SEEDS);
+ Config.NIMBUS_HOST, Config.NIMBUS_SEEDS);
seeds = Lists.newArrayList(conf.get(Config.NIMBUS_HOST).toString());
} else {
- seeds = (List<String>) conf.get(Config.NIMBUS_SEEDS);
+ Object nimbusSeeds = conf.get(Config.NIMBUS_SEEDS);
+ seeds = nimbusSeeds instanceof String[] ? Lists.newArrayList((String[]) nimbusSeeds) : Lists.newArrayList(nimbusSeeds.toString());
--- End diff --
The config is marked as a list of strings.
https://github.com/apache/storm/blob/master/storm-client/src/jvm/org/apache/storm/Config.java#L1049-L1053
But config validation has not run yet. Perhaps we can validate it before we start using it.
---
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] storm issue #2228: STORM-2646 NimbusClient Class cast exception when nimbus ...
Posted by ecararus <gi...@git.apache.org>.
Github user ecararus commented on the issue:
https://github.com/apache/storm/pull/2228
@revans2 This pr will not fully fix my issue however i will not get anymore class cast exception. validation will fail according to [annotation](https://github.com/apache/storm/blob/master/storm-client/src/jvm/org/apache/storm/Config.java#L1059) and [ConfigValidator](https://github.com/apache/storm/blob/master/storm-client/src/jvm/org/apache/storm/validation/ConfigValidation.java#L657)
---
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] storm issue #2228: STORM-2646 NimbusClient Class cast exception when nimbus ...
Posted by ecararus <gi...@git.apache.org>.
Github user ecararus commented on the issue:
https://github.com/apache/storm/pull/2228
Thank you @srdo for suggestion I've created [jira issue](https://issues.apache.org/jira/browse/STORM-2646) also updated PR Title.
Inside my config file i have:
nimbus.seeds= ["somehost.org"]
nimbus.host= somehost.org # temporar fix
nimbus.thrift.port= 66270
more details about project config on jira.
About List<String> i agree with @revans2 about validation and I would like to propose to accept single host also e.g. nimbus.seeds= somehost.org in this way we can support same configuration/behavior like it was with nimbus.host (that might be useful when host is provided by discovery or load distribution service).
---
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] storm pull request #2228: fix class cast exception when nimbus seeds is not ...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/2228#discussion_r128286548
--- Diff: storm-client/src/jvm/org/apache/storm/utils/NimbusClient.java ---
@@ -109,12 +109,13 @@ public static NimbusClient getConfiguredClientAs(Map<String, Object> conf, Strin
}
List<String> seeds;
- if(conf.containsKey(Config.NIMBUS_HOST)) {
+ if (conf.containsKey(Config.NIMBUS_HOST)) {
LOG.warn("Using deprecated config {} for backward compatibility. Please update your storm.yaml so it only has config {}",
- Config.NIMBUS_HOST, Config.NIMBUS_SEEDS);
+ Config.NIMBUS_HOST, Config.NIMBUS_SEEDS);
seeds = Lists.newArrayList(conf.get(Config.NIMBUS_HOST).toString());
} else {
- seeds = (List<String>) conf.get(Config.NIMBUS_SEEDS);
+ Object nimbusSeeds = conf.get(Config.NIMBUS_SEEDS);
+ seeds = nimbusSeeds instanceof String[] ? Lists.newArrayList((String[]) nimbusSeeds) : Lists.newArrayList(nimbusSeeds.toString());
--- End diff --
If this is an administrator configuration setting, can we process the value consistently such that `conf.get(Config.NIMBUS_SEEDS)` always returns a list even if there is only one value. Otherwise, we will have to do this checking at every spot where this value is accessed.
How is this config field validated? Do we allow either a single value or a list?
---
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] storm issue #2228: STORM-2646 NimbusClient Class cast exception when nimbus ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2228
The patch right now looks fine to me, but I don't think it fixes your issue with flux. Looking at the flux code inside FluxParser.java the `${NAME}` substitution is literately a macro substitution that happens before the YAML parsing. So if you want it to be an array you need to do like @srdo said and use `["nb.mycompany.org"]` for nimbus.seeds in your properties file instead of just `"nb.mycompany.org"`.
---
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] storm issue #2228: STORM-2646 NimbusClient Class cast exception when nimbus ...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2228
@ecararus Thanks. I'm not too familiar with Flux, but going by what you listed in the issue, I'm guessing it's a problem where the `["nb.mycompany.org"]` from dev.properties is being substituted into the topology.yaml as a string instead of a list.
Shot in the dark, does your topology.yaml do something like
```
config:
nimbus.seeds: "${nimbus.seeds}"
```
If so, would it work to instead write
```
config:
nimbus.seeds: ${nimbus.seeds}
```
---
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] storm pull request #2228: STORM-2646 NimbusClient Class cast exception when ...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2228#discussion_r128613874
--- Diff: storm-client/src/jvm/org/apache/storm/utils/NimbusClient.java ---
@@ -107,13 +109,14 @@ public static NimbusClient getConfiguredClientAs(Map<String, Object> conf, Strin
}
asUser = (String) conf.get(Config.STORM_DO_AS_USER);
}
+ ConfigValidation.validateFields(conf);
- List<String> seeds;
- if(conf.containsKey(Config.NIMBUS_HOST)) {
+ List<String> seeds = Lists.newArrayList();
--- End diff --
Isn't this list overwritten in both branches below?
---
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] storm pull request #2228: STORM-2646 NimbusClient Class cast exception when ...
Posted by ecararus <gi...@git.apache.org>.
Github user ecararus closed the pull request at:
https://github.com/apache/storm/pull/2228
---
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] storm issue #2228: STORM-2646 NimbusClient Class cast exception when nimbus ...
Posted by ecararus <gi...@git.apache.org>.
Github user ecararus commented on the issue:
https://github.com/apache/storm/pull/2228
Hello all
Thanks for advice i found the issue and fixed, i would like to apologize for misleading by pointing on FluxParser(however propertied.get(key) is returning string thanks to Yaml.load nimbus.seeds is transformed to a list). I'm using [storm-maven-plugin](https://github.com/hubrick/storm-maven-plugin) so the issue was there i'm calling NimbusClient by passing config as:
```
in = new FileInputStream(properiesFileLocation);
Properties prop = new Properties();
prop.load(in);
return config.putAll(prop);
```
I think we can close current issue and apply validation before usage of configuration.
Thanks everybody for help.
---
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] storm pull request #2228: STORM-2646 NimbusClient Class cast exception when ...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/2228#discussion_r128411779
--- Diff: storm-client/src/jvm/org/apache/storm/utils/NimbusClient.java ---
@@ -109,12 +109,13 @@ public static NimbusClient getConfiguredClientAs(Map<String, Object> conf, Strin
}
List<String> seeds;
- if(conf.containsKey(Config.NIMBUS_HOST)) {
+ if (conf.containsKey(Config.NIMBUS_HOST)) {
LOG.warn("Using deprecated config {} for backward compatibility. Please update your storm.yaml so it only has config {}",
- Config.NIMBUS_HOST, Config.NIMBUS_SEEDS);
+ Config.NIMBUS_HOST, Config.NIMBUS_SEEDS);
seeds = Lists.newArrayList(conf.get(Config.NIMBUS_HOST).toString());
} else {
- seeds = (List<String>) conf.get(Config.NIMBUS_SEEDS);
+ Object nimbusSeeds = conf.get(Config.NIMBUS_SEEDS);
+ seeds = nimbusSeeds instanceof String[] ? Lists.newArrayList((String[]) nimbusSeeds) : Lists.newArrayList(nimbusSeeds.toString());
--- End diff --
Yeah I agree. Validation is supposed to prevent this error. The value must be a list of strings.
---
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] storm pull request #2228: STORM-2646 NimbusClient Class cast exception when ...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2228#discussion_r128617905
--- Diff: storm-client/src/jvm/org/apache/storm/utils/NimbusClient.java ---
@@ -107,13 +109,14 @@ public static NimbusClient getConfiguredClientAs(Map<String, Object> conf, Strin
}
asUser = (String) conf.get(Config.STORM_DO_AS_USER);
}
+ ConfigValidation.validateFields(conf);
- List<String> seeds;
- if(conf.containsKey(Config.NIMBUS_HOST)) {
+ List<String> seeds = Lists.newArrayList();
--- End diff --
Sorry, totally spaced and missed the change to L119 :)
---
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] storm issue #2228: fix class cast exception when nimbus seeds is not an arra...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2228
Hi @ecararus. Could you clarify a bit how the changes fix the issue you were seeing, and maybe also which Storm version you were seeing the issue on?
The default setting for nimbus.seeds is ["localhost"] (https://github.com/apache/storm/blob/master/conf/defaults.yaml#L61), which works fine for me when I try it locally, so I don't think there's a problem specifying a single host. We assume the configuration is a String list here https://github.com/apache/storm/blob/master/storm-client/src/jvm/org/apache/storm/Config.java#L1052.
The change in L118 is checking if the config is a String[], which I would assume doesn't work since it is usually a `List<String>`? If you need to be able to write `nimbus.seeds: "host"` instead of `nimbus.seeds: ["host"]`, I think you should update the Config with this annotation https://github.com/apache/storm/blob/master/storm-client/src/jvm/org/apache/storm/Config.java#L1120.
Please also create an issue on JIRA https://issues.apache.org/jira and update your commit message and this PR's title to reflect the issue number (see the other PRs for example of what this should look like). 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 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] storm pull request #2228: STORM-2646 NimbusClient Class cast exception when ...
Posted by ecararus <gi...@git.apache.org>.
Github user ecararus commented on a diff in the pull request:
https://github.com/apache/storm/pull/2228#discussion_r128616112
--- Diff: storm-client/src/jvm/org/apache/storm/utils/NimbusClient.java ---
@@ -107,13 +109,14 @@ public static NimbusClient getConfiguredClientAs(Map<String, Object> conf, Strin
}
asUser = (String) conf.get(Config.STORM_DO_AS_USER);
}
+ ConfigValidation.validateFields(conf);
- List<String> seeds;
- if(conf.containsKey(Config.NIMBUS_HOST)) {
+ List<String> seeds = Lists.newArrayList();
--- End diff --
By default seeds will be empty and will be populated only if NIMBUS_HOST or NIMBUS_SEEDS will be configured. in case if both missing an exception will [be thrown here](https://github.com/apache/storm/blob/master/storm-client/src/jvm/org/apache/storm/utils/NimbusClient.java#L153).
---
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] storm issue #2228: STORM-2646 NimbusClient Class cast exception when nimbus ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2228
@ecararus could you please take a look at https://github.com/revans2/incubator-storm/commit/0978338c7dd7ec23b035abb5f70ce088064bf60b
A lot of it you can ignore, but I added in a test on top of this patch into flux-core that shows that `nimbus.seeds` works (if you set it properly in your properties file).
Specifically the properties file needs to have in it
```
nimbus.seeds: ["HOST"]
```
and not
```
nimbus.seeds: "HOST"
```
When I change it to the latter the test fails with an exception of
```
java.lang.IllegalArgumentException: Field NIMBUS_SEEDS must be an Iterable but was a class java.lang.String
at org.apache.storm.validation.ConfigValidationUtils$2.validateField(ConfigValidationUtils.java:125)
at org.apache.storm.validation.ConfigValidationUtils$NestableFieldValidator.validateField(ConfigValidationUtils.java:48)
at org.apache.storm.validation.ConfigValidation$ListEntryTypeValidator.validateField(ConfigValidation.java:311)
at org.apache.storm.validation.ConfigValidation$ListEntryTypeValidator.validateField(ConfigValidation.java:306)
at org.apache.storm.validation.ConfigValidation.validateField(ConfigValidation.java:697)
at org.apache.storm.validation.ConfigValidation.validateFields(ConfigValidation.java:750)
at org.apache.storm.validation.ConfigValidation.validateFields(ConfigValidation.java:711)
at org.apache.storm.flux.parser.FluxParserTest.testCofigParamListParser(FluxParserTest.java:43)
```
---
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] storm issue #2228: STORM-2646 NimbusClient Class cast exception when nimbus ...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2228
I'm a little confused why it doesn't work for you @ecararus. I added a bit to a Flux test to verify that it is possible to pass a list from a properties file into the Flux yaml file via FluxParser, both on master and 1.0.x. Does the diff at https://github.com/apache/storm/pull/2232 accurately reflect what your properties and yaml files contain?
---
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.
---