[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user asfgit closed the pull request at: https://github.com/apache/nifi-minifi/pull/45 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r85604460 --- Diff: minifi-toolkit/minifi-toolkit-configuration/src/main/java/org/apache/nifi/minifi/toolkit/configuration/ConfigMain.java --- @@ -336,6 +329,16 @@ public int transform(String[] args) { return SUCCESS; } +protected void validateAndPrintIssues(ConfigSchema configSchema) { +if (!configSchema.isValid()) { +System.out.println("There are validation errors with the template, still outputting YAML but it will need to be edited."); + configSchema.getValidationIssues().forEach(System.out::println); --- End diff -- Yup the first print is correct (was just including it to show what is wrong with the config). The print out for the upgrade could be more user friendly something like: ` There were validation errors prior to upgrading: After upgrading it has these validation errors: Any validation errors after upgrading will need to be addressed prior to using the config. ` In its current state it looks like the upgraded template has validation errors for both destination ids and destination name (which should be mutually exclusive). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user brosander commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r85601256 --- Diff: minifi-commons/minifi-commons-schema/src/main/java/org/apache/nifi/minifi/commons/schema/serialization/SchemaLoader.java --- @@ -78,6 +80,27 @@ public static ConfigSchema loadConfigSchemaFromYaml(MapyamlAsMa public static ConvertableSchema loadConvertableSchemaFromYaml(Map yamlAsMap) throws SchemaLoaderException { String version = String.valueOf(yamlAsMap.get(ConfigSchema.VERSION)); +if (StringUtil.isNullOrEmpty(version) || String.valueOf((Object) null).equals(version)) { --- End diff -- @JPercivall sorry, I thought you were wanting behavior like that in regards to [your comment above.](https://github.com/apache/nifi-minifi/pull/45#discussion_r85137468) I must've misread the comment, now it's looking more like it may have been automatically addressed by the other validation change which clears the errors before converting. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r85600674 --- Diff: minifi-commons/minifi-commons-schema/src/main/java/org/apache/nifi/minifi/commons/schema/serialization/SchemaLoader.java --- @@ -78,6 +80,27 @@ public static ConfigSchema loadConfigSchemaFromYaml(MapyamlAsMa public static ConvertableSchema loadConvertableSchemaFromYaml(Map yamlAsMap) throws SchemaLoaderException { String version = String.valueOf(yamlAsMap.get(ConfigSchema.VERSION)); +if (StringUtil.isNullOrEmpty(version) || String.valueOf((Object) null).equals(version)) { --- End diff -- @brosander what was the motivation to move all of this logging here and potentially change which yaml version gets used? I preferred keeping the version as a required property and assuming that null and "" were v1 (makes things much simpler in the long run). A side effect of the logging is weird messages when doing simple validation or conversion using the toolkit (test.yml has no config version listed): ![screen shot 2016-10-28 at 4 02 51 pm](https://cloud.githubusercontent.com/assets/11302527/19820530/cc7af110-9d27-11e6-8e9f-279f6a79a091.png) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user brosander commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r85150769 --- Diff: minifi-commons/minifi-commons-schema/src/main/java/org/apache/nifi/minifi/commons/schema/serialization/SchemaLoader.java --- @@ -30,13 +32,13 @@ import java.util.stream.Collectors; public class SchemaLoader { --- End diff -- @JPercivall it would be pretty straightforward to filter null or empty values from that printout which is the only place that map is enumerated. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user brosander commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r85150335 --- Diff: minifi-toolkit/minifi-toolkit-configuration/src/main/java/org/apache/nifi/minifi/toolkit/configuration/ConfigMain.java --- @@ -235,62 +252,127 @@ private static void addRemoteProcessGroupPortDTOs(Mapconnectabl } } +public int upgrade(String[] args) { +if (args.length != 3) { +printUgradeUsage(); +return ERR_INVALID_ARGS; +} + +ConfigSchema configSchema = null; +try (InputStream inputStream = pathInputStreamFactory.create(args[1])) { +try { +configSchema = SchemaLoader.loadConfigSchemaFromYaml(inputStream); +} catch (IOException|SchemaLoaderException e) { +return handleErrorLoadingConfiguration(e, ConfigMain::printUgradeUsage); +} +} catch (FileNotFoundException e) { +return handleErrorOpeningInput(args[1], ConfigMain::printUgradeUsage, e); +} catch (IOException e) { +handleErrorClosingInput(e); +} + +try (OutputStream fileOutputStream = pathOutputStreamFactory.create(args[2])) { +try { +SchemaSaver.saveConfigSchema(configSchema, fileOutputStream); +} catch (IOException e) { +return handleErrorSavingCofiguration(e); +} +} catch (FileNotFoundException e) { +return handleErrorOpeningOutput(args[2], ConfigMain::printUgradeUsage, e); +} catch (IOException e) { +handleErrorClosingOutput(e); +} + +return SUCCESS; --- End diff -- Good point, adding --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user brosander commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r85147924 --- Diff: minifi-toolkit/minifi-toolkit-configuration/src/main/java/org/apache/nifi/minifi/toolkit/configuration/ConfigMain.java --- @@ -96,31 +102,39 @@ public int validate(String[] args) { } try (InputStream inputStream = pathInputStreamFactory.create(args[1])) { try { -ConfigSchema configSchema = SchemaLoader.loadConfigSchemaFromYaml(inputStream); +ConvertableSchema configSchema = SchemaLoader.loadConvertableSchemaFromYaml(inputStream); +boolean valid = true; if (!configSchema.isValid()) { + System.out.println(FOUND_THE_FOLLOWING_ERRORS_WHEN_PARSING_THE_TEMPLATE_ACCORDING_TO_ITS_VERSION); --- End diff -- Good point, I'll add that. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r85137736 --- Diff: minifi-toolkit/minifi-toolkit-configuration/src/main/java/org/apache/nifi/minifi/toolkit/configuration/ConfigMain.java --- @@ -96,31 +102,39 @@ public int validate(String[] args) { } try (InputStream inputStream = pathInputStreamFactory.create(args[1])) { try { -ConfigSchema configSchema = SchemaLoader.loadConfigSchemaFromYaml(inputStream); +ConvertableSchema configSchema = SchemaLoader.loadConvertableSchemaFromYaml(inputStream); +boolean valid = true; if (!configSchema.isValid()) { + System.out.println(FOUND_THE_FOLLOWING_ERRORS_WHEN_PARSING_THE_TEMPLATE_ACCORDING_TO_ITS_VERSION); configSchema.getValidationIssues().forEach(s -> System.out.println(s)); System.out.println(); -return ERR_INVALID_CONFIG; -} else { +valid = false; +configSchema.clearValidationIssues(); +} + +ConfigSchema currentSchema = configSchema.convert(); +if (!currentSchema.isValid()) { + System.out.println(WHEN_CONVERTING_TO_LATEST_VERSION_FOUND_THE_FOLLOWING_ERRORS); --- End diff -- This should state what the latest version for this toolkit is. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r85137468 --- Diff: minifi-toolkit/minifi-toolkit-configuration/src/main/java/org/apache/nifi/minifi/toolkit/configuration/ConfigMain.java --- @@ -96,31 +102,39 @@ public int validate(String[] args) { } try (InputStream inputStream = pathInputStreamFactory.create(args[1])) { try { -ConfigSchema configSchema = SchemaLoader.loadConfigSchemaFromYaml(inputStream); +ConvertableSchema configSchema = SchemaLoader.loadConvertableSchemaFromYaml(inputStream); +boolean valid = true; if (!configSchema.isValid()) { + System.out.println(FOUND_THE_FOLLOWING_ERRORS_WHEN_PARSING_THE_TEMPLATE_ACCORDING_TO_ITS_VERSION); configSchema.getValidationIssues().forEach(s -> System.out.println(s)); System.out.println(); -return ERR_INVALID_CONFIG; -} else { +valid = false; +configSchema.clearValidationIssues(); +} + +ConfigSchema currentSchema = configSchema.convert(); --- End diff -- Something that's a bit confusing to the user, I had a valid v2 config file and commented out the version line. When I did this I properly saw that there were validation errors when validating it for it's current version (v1) but didn't expect to see errors when "converting" it to the current version. I completely understand why (the connection ids were not ingested) but maybe either the second check needs to change or a third where it explicitly tries to load it with the latest schema. Or I could be completely off base, lol, let me know what you think. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r85137652 --- Diff: minifi-toolkit/minifi-toolkit-configuration/src/main/java/org/apache/nifi/minifi/toolkit/configuration/ConfigMain.java --- @@ -96,31 +102,39 @@ public int validate(String[] args) { } try (InputStream inputStream = pathInputStreamFactory.create(args[1])) { try { -ConfigSchema configSchema = SchemaLoader.loadConfigSchemaFromYaml(inputStream); +ConvertableSchema configSchema = SchemaLoader.loadConvertableSchemaFromYaml(inputStream); +boolean valid = true; if (!configSchema.isValid()) { + System.out.println(FOUND_THE_FOLLOWING_ERRORS_WHEN_PARSING_THE_TEMPLATE_ACCORDING_TO_ITS_VERSION); --- End diff -- This should state what the version of config file is. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r85142121 --- Diff: minifi-commons/minifi-commons-schema/src/main/java/org/apache/nifi/minifi/commons/schema/serialization/SchemaLoader.java --- @@ -30,13 +32,13 @@ import java.util.stream.Collectors; public class SchemaLoader { --- End diff -- When you attempt to load a schema version that's not listed you see this error message (gotten via config.sh validate): YAML configuration version 3 not supported. Supported versions: , 1, 2, null) "" and "Null" are more implementation details that the user doesn't need to know about. Somewhere we should say that a missing version number = v1. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r85127907 --- Diff: minifi-toolkit/minifi-toolkit-configuration/src/main/java/org/apache/nifi/minifi/toolkit/configuration/ConfigMain.java --- @@ -235,62 +252,127 @@ private static void addRemoteProcessGroupPortDTOs(Mapconnectabl } } +public int upgrade(String[] args) { +if (args.length != 3) { +printUgradeUsage(); +return ERR_INVALID_ARGS; +} + +ConfigSchema configSchema = null; +try (InputStream inputStream = pathInputStreamFactory.create(args[1])) { +try { +configSchema = SchemaLoader.loadConfigSchemaFromYaml(inputStream); +} catch (IOException|SchemaLoaderException e) { +return handleErrorLoadingConfiguration(e, ConfigMain::printUgradeUsage); +} +} catch (FileNotFoundException e) { +return handleErrorOpeningInput(args[1], ConfigMain::printUgradeUsage, e); +} catch (IOException e) { +handleErrorClosingInput(e); +} + +try (OutputStream fileOutputStream = pathOutputStreamFactory.create(args[2])) { +try { +SchemaSaver.saveConfigSchema(configSchema, fileOutputStream); +} catch (IOException e) { +return handleErrorSavingCofiguration(e); +} +} catch (FileNotFoundException e) { +return handleErrorOpeningOutput(args[2], ConfigMain::printUgradeUsage, e); +} catch (IOException e) { +handleErrorClosingOutput(e); +} + +return SUCCESS; --- End diff -- This should run a validate on the resulting upgraded file. I ran a test where I upgraded a flow with a connection that was missing a destination and I was confused when it didn't tell there were problems. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r85128182 --- Diff: minifi-toolkit/minifi-toolkit-configuration/src/main/java/org/apache/nifi/minifi/toolkit/configuration/ConfigMain.java --- @@ -235,62 +252,127 @@ private static void addRemoteProcessGroupPortDTOs(Mapconnectabl } } +public int upgrade(String[] args) { +if (args.length != 3) { +printUgradeUsage(); +return ERR_INVALID_ARGS; +} + +ConfigSchema configSchema = null; +try (InputStream inputStream = pathInputStreamFactory.create(args[1])) { +try { +configSchema = SchemaLoader.loadConfigSchemaFromYaml(inputStream); --- End diff -- After ingesting the config, it should check if it is already the latest version. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user brosander commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r84531003 --- Diff: minifi-commons/minifi-commons-schema/src/main/java/org/apache/nifi/minifi/commons/schema/ConfigSchema.java --- @@ -142,128 +125,10 @@ public ConfigSchema(Map map) { } } -protected List getProcessorSchemas(List processorMaps) { -if (processorMaps == null) { -return null; -} -List processors = convertListToType(processorMaps, "processor", ProcessorSchema.class, PROCESSORS_KEY); - -MapidMap = processors.stream().map(ProcessorSchema::getId).filter( -s -> !StringUtil.isNullOrEmpty(s)).collect(Collectors.toMap(Function.identity(), s -> 2, Integer::compareTo)); - -// Set unset ids -processors.stream().filter(connection -> StringUtil.isNullOrEmpty(connection.getId())).forEachOrdered(processor -> processor.setId(getUniqueId(idMap, processor.getName(; - -return processors; -} - -protected List getConnectionSchemas(List connectionMaps) { -if (connectionMaps == null) { -return null; -} -List connections = convertListToType(connectionMaps, "connection", ConnectionSchema.class, CONNECTIONS_KEY); -Map idMap = connections.stream().map(ConnectionSchema::getId).filter( -s -> !StringUtil.isNullOrEmpty(s)).collect(Collectors.toMap(Function.identity(), s -> 2, Integer::compareTo)); - -Map processorNameToIdMap = new HashMap<>(); - -// We can't look up id by name for names that appear more than once -Set duplicateProcessorNames = new HashSet<>(); - -List processors = getProcessors(); -if (processors != null) { -processors.stream().forEachOrdered(p -> processorNameToIdMap.put(p.getName(), p.getId())); - -Set processorNames = new HashSet<>(); - processors.stream().map(ProcessorSchema::getName).forEachOrdered(n -> { -if (!processorNames.add(n)) { -duplicateProcessorNames.add(n); -} -}); -} - -Set remoteInputPortIds = new HashSet<>(); -List remoteProcessingGroups = getRemoteProcessingGroups(); -if (remoteProcessingGroups != null) { - remoteInputPortIds.addAll(remoteProcessingGroups.stream().filter(r -> r.getInputPorts() != null) -.flatMap(r -> r.getInputPorts().stream()).map(RemoteInputPortSchema::getId).collect(Collectors.toSet())); -} - -Set problematicDuplicateNames = new HashSet<>(); -Set missingProcessorNames = new HashSet<>(); -// Set unset ids -connections.stream().filter(connection -> StringUtil.isNullOrEmpty(connection.getId())).forEachOrdered(connection -> connection.setId(getUniqueId(idMap, connection.getName(; - -connections.stream().filter(connection -> StringUtil.isNullOrEmpty(connection.getSourceId())).forEach(connection -> { -String sourceName = connection.getSourceName(); -if (remoteInputPortIds.contains(sourceName)) { -connection.setSourceId(sourceName); -} else { -if (duplicateProcessorNames.contains(sourceName)) { -problematicDuplicateNames.add(sourceName); -} -String sourceId = processorNameToIdMap.get(sourceName); -if (StringUtil.isNullOrEmpty(sourceId)) { -missingProcessorNames.add(sourceName); -} else { -connection.setSourceId(sourceId); -} -} -}); - -connections.stream().filter(connection -> StringUtil.isNullOrEmpty(connection.getDestinationId())) -.forEach(connection -> { -String destinationName = connection.getDestinationName(); -if (remoteInputPortIds.contains(destinationName)) { -connection.setDestinationId(destinationName); -} else { -if (duplicateProcessorNames.contains(destinationName)) { -problematicDuplicateNames.add(destinationName); -} -String destinationId = processorNameToIdMap.get(destinationName); -if (StringUtil.isNullOrEmpty(destinationId)) { -missingProcessorNames.add(destinationName); -} else { -
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r84530471 --- Diff: minifi-commons/minifi-commons-schema/src/main/java/org/apache/nifi/minifi/commons/schema/ConnectionSchema.java --- @@ -60,30 +53,11 @@ public ConnectionSchema(Map map) { super(map, CONNECTIONS_KEY); sourceId = getOptionalKeyAsType(map, SOURCE_ID_KEY, String.class, CONNECTIONS_KEY, ""); --- End diff -- Ah I see, could you just add a comment above these two "optional" calls explaining that? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user brosander commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r84529741 --- Diff: minifi-commons/minifi-commons-schema/src/main/java/org/apache/nifi/minifi/commons/schema/serialization/SchemaLoader.java --- @@ -30,13 +32,13 @@ import java.util.stream.Collectors; public class SchemaLoader { -private static final Map> configSchemaFactories = initConfigSchemaFactories(); +private static final Map > configSchemaFactories = initConfigSchemaFactories(); -private static Map > initConfigSchemaFactories() { -Map > result = new HashMap<>(); -result.put(String.valueOf((Object)null), ConfigSchema::new); -result.put("", ConfigSchema::new); -result.put("1", ConfigSchema::new); --- End diff -- Good point, I'll take a stab at updating the doc --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user brosander commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r84529673 --- Diff: minifi-commons/minifi-commons-schema/src/main/java/org/apache/nifi/minifi/commons/schema/ConnectionSchema.java --- @@ -60,30 +53,11 @@ public ConnectionSchema(Map map) { super(map, CONNECTIONS_KEY); sourceId = getOptionalKeyAsType(map, SOURCE_ID_KEY, String.class, CONNECTIONS_KEY, ""); --- End diff -- It is treated as required in the getValidationIssues implementation. I needed a chance to set it when upconverting before the validation occurs. I could change that to map manipulation before instantiating the ConnectionSchema but that felt worse. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user brosander commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r84529455 --- Diff: minifi-toolkit/minifi-toolkit-configuration/src/main/java/org/apache/nifi/minifi/toolkit/configuration/ConfigMain.java --- @@ -96,13 +99,21 @@ public int validate(String[] args) { } try (InputStream inputStream = pathInputStreamFactory.create(args[1])) { try { -ConfigSchema configSchema = SchemaLoader.loadConfigSchemaFromYaml(inputStream); +ConvertableSchema configSchema = SchemaLoader.loadConvertableSchemaFromYaml(inputStream); if (!configSchema.isValid()) { configSchema.getValidationIssues().forEach(s -> System.out.println(s)); System.out.println(); return ERR_INVALID_CONFIG; } else { - System.out.println(NO_VALIDATION_ERRORS_FOUND_IN_TEMPLATE); +ConfigSchema currentSchema = configSchema.convert(); --- End diff -- I think it would, good call :) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r84527469 --- Diff: minifi-commons/minifi-commons-schema/src/main/java/org/apache/nifi/minifi/commons/schema/ConnectionSchema.java --- @@ -60,30 +53,11 @@ public ConnectionSchema(Map map) { super(map, CONNECTIONS_KEY); sourceId = getOptionalKeyAsType(map, SOURCE_ID_KEY, String.class, CONNECTIONS_KEY, ""); --- End diff -- I believe this should now be required instead of optional (since there is no id vs name confusion) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r84527193 --- Diff: minifi-commons/minifi-commons-schema/src/main/java/org/apache/nifi/minifi/commons/schema/ConfigSchema.java --- @@ -142,128 +125,10 @@ public ConfigSchema(Map map) { } } -protected List getProcessorSchemas(List processorMaps) { -if (processorMaps == null) { -return null; -} -List processors = convertListToType(processorMaps, "processor", ProcessorSchema.class, PROCESSORS_KEY); - -MapidMap = processors.stream().map(ProcessorSchema::getId).filter( -s -> !StringUtil.isNullOrEmpty(s)).collect(Collectors.toMap(Function.identity(), s -> 2, Integer::compareTo)); - -// Set unset ids -processors.stream().filter(connection -> StringUtil.isNullOrEmpty(connection.getId())).forEachOrdered(processor -> processor.setId(getUniqueId(idMap, processor.getName(; - -return processors; -} - -protected List getConnectionSchemas(List connectionMaps) { -if (connectionMaps == null) { -return null; -} -List connections = convertListToType(connectionMaps, "connection", ConnectionSchema.class, CONNECTIONS_KEY); -Map idMap = connections.stream().map(ConnectionSchema::getId).filter( -s -> !StringUtil.isNullOrEmpty(s)).collect(Collectors.toMap(Function.identity(), s -> 2, Integer::compareTo)); - -Map processorNameToIdMap = new HashMap<>(); - -// We can't look up id by name for names that appear more than once -Set duplicateProcessorNames = new HashSet<>(); - -List processors = getProcessors(); -if (processors != null) { -processors.stream().forEachOrdered(p -> processorNameToIdMap.put(p.getName(), p.getId())); - -Set processorNames = new HashSet<>(); - processors.stream().map(ProcessorSchema::getName).forEachOrdered(n -> { -if (!processorNames.add(n)) { -duplicateProcessorNames.add(n); -} -}); -} - -Set remoteInputPortIds = new HashSet<>(); -List remoteProcessingGroups = getRemoteProcessingGroups(); -if (remoteProcessingGroups != null) { - remoteInputPortIds.addAll(remoteProcessingGroups.stream().filter(r -> r.getInputPorts() != null) -.flatMap(r -> r.getInputPorts().stream()).map(RemoteInputPortSchema::getId).collect(Collectors.toSet())); -} - -Set problematicDuplicateNames = new HashSet<>(); -Set missingProcessorNames = new HashSet<>(); -// Set unset ids -connections.stream().filter(connection -> StringUtil.isNullOrEmpty(connection.getId())).forEachOrdered(connection -> connection.setId(getUniqueId(idMap, connection.getName(; - -connections.stream().filter(connection -> StringUtil.isNullOrEmpty(connection.getSourceId())).forEach(connection -> { -String sourceName = connection.getSourceName(); -if (remoteInputPortIds.contains(sourceName)) { -connection.setSourceId(sourceName); -} else { -if (duplicateProcessorNames.contains(sourceName)) { -problematicDuplicateNames.add(sourceName); -} -String sourceId = processorNameToIdMap.get(sourceName); -if (StringUtil.isNullOrEmpty(sourceId)) { -missingProcessorNames.add(sourceName); -} else { -connection.setSourceId(sourceId); -} -} -}); - -connections.stream().filter(connection -> StringUtil.isNullOrEmpty(connection.getDestinationId())) -.forEach(connection -> { -String destinationName = connection.getDestinationName(); -if (remoteInputPortIds.contains(destinationName)) { -connection.setDestinationId(destinationName); -} else { -if (duplicateProcessorNames.contains(destinationName)) { -problematicDuplicateNames.add(destinationName); -} -String destinationId = processorNameToIdMap.get(destinationName); -if (StringUtil.isNullOrEmpty(destinationId)) { -missingProcessorNames.add(destinationName); -} else {
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r84528235 --- Diff: minifi-commons/minifi-commons-schema/src/main/java/org/apache/nifi/minifi/commons/schema/serialization/SchemaLoader.java --- @@ -30,13 +32,13 @@ import java.util.stream.Collectors; public class SchemaLoader { -private static final Map> configSchemaFactories = initConfigSchemaFactories(); +private static final Map > configSchemaFactories = initConfigSchemaFactories(); -private static Map > initConfigSchemaFactories() { -Map > result = new HashMap<>(); -result.put(String.valueOf((Object)null), ConfigSchema::new); -result.put("", ConfigSchema::new); -result.put("1", ConfigSchema::new); --- End diff -- The changes in the Config schemas need to be documented. Could you add a section to the admin guide (the Config Schema portion) that explains the versioning scheme as well as what changed in v2? I am suggesting to put it in the docs instead of a migration guidance for a release because working with multiple different version of MiNiFi and configs will be an everyday task (esp when C is added). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r84488138 --- Diff: minifi-toolkit/minifi-toolkit-configuration/src/main/java/org/apache/nifi/minifi/toolkit/configuration/ConfigMain.java --- @@ -96,13 +99,21 @@ public int validate(String[] args) { } try (InputStream inputStream = pathInputStreamFactory.create(args[1])) { try { -ConfigSchema configSchema = SchemaLoader.loadConfigSchemaFromYaml(inputStream); +ConvertableSchema configSchema = SchemaLoader.loadConvertableSchemaFromYaml(inputStream); if (!configSchema.isValid()) { configSchema.getValidationIssues().forEach(s -> System.out.println(s)); System.out.println(); return ERR_INVALID_CONFIG; } else { - System.out.println(NO_VALIDATION_ERRORS_FOUND_IN_TEMPLATE); +ConfigSchema currentSchema = configSchema.convert(); --- End diff -- Would it make sense to convert and show any validation errors even if it's not valid in the old version? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
Github user JPercivall commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/45#discussion_r84527495 --- Diff: minifi-commons/minifi-commons-schema/src/main/java/org/apache/nifi/minifi/commons/schema/ConnectionSchema.java --- @@ -60,30 +53,11 @@ public ConnectionSchema(Map map) { super(map, CONNECTIONS_KEY); sourceId = getOptionalKeyAsType(map, SOURCE_ID_KEY, String.class, CONNECTIONS_KEY, ""); -if (StringUtil.isNullOrEmpty(sourceId)) { -sourceName = getRequiredKeyAsType(map, SOURCE_NAME_KEY, String.class, CONNECTIONS_KEY); -} - -String sourceRelationshipName = getOptionalKeyAsType(map, SOURCE_RELATIONSHIP_NAME_KEY, String.class, CONNECTIONS_KEY, null); -if (StringUtil.isNullOrEmpty(sourceRelationshipName)) { -sourceRelationshipNames = getOptionalKeyAsType(map, SOURCE_RELATIONSHIP_NAMES_KEY, List.class, CONNECTIONS_KEY, new ArrayList()); -if (sourceRelationshipNames.isEmpty()) { - addValidationIssue(getIssueText(SOURCE_RELATIONSHIP_NAMES_KEY, CONNECTIONS_KEY, "expected at least one relationship to be specified")); -} -} else { -if (map.containsKey(SOURCE_RELATIONSHIP_NAMES_KEY)) { -addValidationIssue("Only one of " + SOURCE_RELATIONSHIP_NAME_KEY + ", " + SOURCE_RELATIONSHIP_NAMES_KEY + " should be set per connection. Found both on " -+ (StringUtil.isNullOrEmpty(getName()) ? getId() : getName())); -sourceRelationshipNames = getRequiredKeyAsType(map, SOURCE_RELATIONSHIP_NAMES_KEY, List.class, CONNECTIONS_KEY); -} else { -sourceRelationshipNames = new ArrayList<>(Arrays.asList(sourceRelationshipName)); -} +sourceRelationshipNames = getOptionalKeyAsType(map, SOURCE_RELATIONSHIP_NAMES_KEY, List.class, CONNECTIONS_KEY, new ArrayList<>()); +if (sourceRelationshipNames.isEmpty()) { +addValidationIssue("Expected at least one value in " + SOURCE_RELATIONSHIP_NAMES_KEY + " for " + CONNECTIONS_KEY + " " + getName()); } - destinationId = getOptionalKeyAsType(map, DESTINATION_ID_KEY, String.class, CONNECTIONS_KEY, ""); --- End diff -- Same comment as on source, I think this should be "required". --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi-minifi pull request #45: MINIFI-117 - Maintainable Configuration Versio...
GitHub user brosander opened a pull request: https://github.com/apache/nifi-minifi/pull/45 MINIFI-117 - Maintainable Configuration Versioning You can merge this pull request into a Git repository by running: $ git pull https://github.com/brosander/nifi-minifi MINIFI-117-3 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi-minifi/pull/45.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 #45 commit 1f8f6e605dce1ab5dfab10f7a267e46c7c727bfe Author: Bryan RosanderDate: 2016-10-07T17:23:21Z MINIFI-117 - Maintainable Configuration Versioning --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---