[
https://issues.apache.org/jira/browse/METRON-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16273452#comment-16273452
]
ASF GitHub Bot commented on METRON-1336:
----------------------------------------
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/851#discussion_r154202685
--- Diff:
metron-platform/metron-common/src/main/java/org/apache/metron/common/configuration/ConfigurationsUtils.java
---
@@ -343,25 +343,57 @@ public static void uploadConfigsToZookeeper(String
rootFilePath, CuratorFramewor
* @param type config type to upload configs for
* @param configName specific config under the specified config type
*/
- public static void uploadConfigsToZookeeper(String rootFilePath,
CuratorFramework client,
- ConfigurationType type, Optional<String> configName) throws
Exception {
+ public static void uploadConfigsToZookeeper(
+ String rootFilePath,
+ CuratorFramework client,
+ ConfigurationType type,
+ Optional<String> configName) throws Exception {
+
switch (type) {
+
case GLOBAL:
final byte[] globalConfig = readGlobalConfigFromFile(rootFilePath);
if (globalConfig.length > 0) {
setupStellarStatically(client, Optional.of(new
String(globalConfig)));
writeGlobalConfigToZookeeper(globalConfig, client);
}
break;
- case PARSER: // intentional pass-through
- case ENRICHMENT: // intentional pass-through
- case INDEXING:
- Map<String, byte[]> sensorIndexingConfigs =
readSensorConfigsFromFile(rootFilePath, type,
- configName);
- for (String sensorType : sensorIndexingConfigs.keySet()) {
- writeConfigToZookeeper(type, configName,
sensorIndexingConfigs.get(sensorType), client);
+
+ case PARSER: {
+ Map<String, byte[]> configs =
readSensorConfigsFromFile(rootFilePath, PARSER, configName);
--- End diff --
These cases look like they are cut and pasted which seems like code smell
to me and might be a maintenance issue. Can we extract the common code for
Parser, Enrichment, and Indexing into a separate function that is called here?
Perhaps something like:
```
void writeSensorConfigs(ConfigurationType type, Optional<String>
configName, BiFunction<String, byte[], Void> callback) {
Map<String, byte[]> configs = readSensorConfigsFromFile(rootFilePath,
type, configName);
for(String sensorType : configs.keySet()) {
byte[] configData = configs.get(sensorType);
callback.apply(sensorType, configData);
}
}
```
which could be called from this case via `writeSensorConfigs(PARSER,
configName, (sensorType, configData) ->
writeSensorParserConfigToZookeeper(sensorType, configData, client));`
Take the above as a very rough suggestion and you can feel to abstract it
however you wish.
> Patching Can Result in Bad Configuration
> ----------------------------------------
>
> Key: METRON-1336
> URL: https://issues.apache.org/jira/browse/METRON-1336
> Project: Metron
> Issue Type: Bug
> Reporter: Nick Allen
> Assignee: Nick Allen
> Fix For: 0.4.1
>
>
> When applying a patch with `zk_load_configs` the resulting configuration can
> be invalid. The resulting configuration should be validated so that a patch
> can never result in an invalid configuration.
> For example, applying the following patch with `zk_load_config` to the
> Profiler yields a broken Profiler configuration.
> {code}
> [
> {
> "path":"profiles",
> "value":{
> "profile":"sketchy_mad",
> "onlyif":"true",
> "update":{
> "s":"OUTLIER_MAD_ADD(s, value)"
> },
> "init":{
> "s":"OUTLIER_MAD_STATE_MERGE(PROFILE_GET('sketchy_mad','global',
> PROFILE_FIXED(5, 'MINUTES')))"
> },
> "foreach":"'global'",
> "result":"s"
> },
> "op":"add"
> }
> ]
> {code}
> The broken configuration is only discovered after dumping the configuration.
> {code}
> $ bin/zk_load_configs.sh -z $ZOOKEEPER -m DUMP -c PROFILER
> Exception in thread "main" java.lang.RuntimeException: Unable to load {
> "profiles" : {
> "profile" : "sketchy_mad",
> "onlyif" : "true",
> "update" : {
> "s" : "OUTLIER_MAD_ADD(s, value)"
> },
> "init" : {
> "s" : "OUTLIER_MAD_STATE_MERGE(PROFILE_GET('sketchy_mad','global',
> PROFILE_FIXED(5, 'MINUTES')))"
> },
> "foreach" : "'global'",
> "result" : "s"
> }
> }
> at
> org.apache.metron.common.configuration.ConfigurationType.lambda$static$4(ConfigurationType.java:68)
> at
> org.apache.metron.common.configuration.ConfigurationType.deserialize(ConfigurationType.java:93)
> at
> org.apache.metron.common.configuration.ConfigurationsUtils.lambda$dumpConfigs$6(ConfigurationsUtils.java:621)
> at
> org.apache.metron.common.configuration.ConfigurationsUtils.visitConfigs(ConfigurationsUtils.java:575)
> at
> org.apache.metron.common.configuration.ConfigurationsUtils.dumpConfigs(ConfigurationsUtils.java:619)
> at
> org.apache.metron.common.cli.ConfigurationManager.dump(ConfigurationManager.java:189)
> at
> org.apache.metron.common.cli.ConfigurationManager.run(ConfigurationManager.java:268)
> at
> org.apache.metron.common.cli.ConfigurationManager.run(ConfigurationManager.java:243)
> at
> org.apache.metron.common.cli.ConfigurationManager.main(ConfigurationManager.java:355)
> Caused by: org.apache.metron.jackson.databind.JsonMappingException: Can not
> deserialize instance of java.util.ArrayList out of START_OBJECT token
> at [Source: {
> "profiles" : {
> "profile" : "sketchy_mad",
> "onlyif" : "true",
> "update" : {
> "s" : "OUTLIER_MAD_ADD(s, value)"
> },
> "init" : {
> "s" : "OUTLIER_MAD_STATE_MERGE(PROFILE_GET('sketchy_mad','global',
> PROFILE_FIXED(5, 'MINUTES')))"
> },
> "foreach" : "'global'",
> "result" : "s"
> }
> }; line: 2, column: 16] (through reference chain:
> org.apache.metron.common.configuration.profiler.ProfilerConfig["profiles"])
> at
> org.apache.metron.jackson.databind.JsonMappingException.from(JsonMappingException.java:255)
> at
> org.apache.metron.jackson.databind.DeserializationContext.mappingException(DeserializationContext.java:971)
> at
> org.apache.metron.jackson.databind.DeserializationContext.mappingException(DeserializationContext.java:967)
> at
> org.apache.metron.jackson.databind.deser.std.CollectionDeserializer.handleNonArray(CollectionDeserializer.java:327)
> at
> org.apache.metron.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:259)
> at
> org.apache.metron.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:249)
> at
> org.apache.metron.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:26)
> at
> org.apache.metron.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:490)
> at
> org.apache.metron.jackson.databind.deser.impl.MethodProperty.deserializeAndSet(MethodProperty.java:95)
> at
> org.apache.metron.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:260)
> at
> org.apache.metron.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:125)
> at
> org.apache.metron.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:3807)
> at
> org.apache.metron.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2797)
> at org.apache.metron.common.utils.JSONUtils.load(JSONUtils.java:79)
> at
> org.apache.metron.common.configuration.ConfigurationType.lambda$static$4(ConfigurationType.java:66)
> ... 8 more
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)