[GitHub] [nifi] mattyb149 commented on a change in pull request #3953: NIFI-5901 Added JSON/JSONB support to PutDatabaseRecord

2020-01-08 Thread GitBox
mattyb149 commented on a change in pull request #3953: NIFI-5901 Added 
JSON/JSONB support to PutDatabaseRecord
URL: https://github.com/apache/nifi/pull/3953#discussion_r364540127
 
 

 ##
 File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutDatabaseRecord.java
 ##
 @@ -252,6 +258,16 @@
 .defaultValue("false")
 .build();
 
+static final PropertyDescriptor MAP_RECORD_TO_JSON = new 
PropertyDescriptor.Builder()
+.name("put-db-record-map-record-to-json")
+.displayName("Map \"record\" types to JSON")
 
 Review comment:
   I don't see where MapRecord converts Record to JSON, I thought you were 
using Jackson's ObjectMapper for that? For XML I'm leery of JAXB as I guess 
that has Java 9+ and/or classpath issues, but I wonder if XStream is an option. 
Also I don't know if external systems take YAML these days but the idea was to 
offer the capability to support various structured data formats. If everything 
but JSON is a non-starter then I'm fine with leaving it a boolean.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [nifi] mattyb149 commented on a change in pull request #3953: NIFI-5901 Added JSON/JSONB support to PutDatabaseRecord

2020-01-06 Thread GitBox
mattyb149 commented on a change in pull request #3953: NIFI-5901 Added 
JSON/JSONB support to PutDatabaseRecord
URL: https://github.com/apache/nifi/pull/3953#discussion_r363445914
 
 

 ##
 File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutDatabaseRecord.java
 ##
 @@ -1042,7 +1072,22 @@ private static String normalizeColumnName(final String 
colName, final boolean tr
 return colName == null ? null : (translateColumnNames ? 
colName.toUpperCase().replace("_", "") : colName);
 }
 
+private static final ObjectMapper MAPPER = new ObjectMapper();
+
+private static String convertMapRecord(Record record) {
 
 Review comment:
   If we use a set of options for what to convert the record to (see my other 
comment), this method should be named accordingly for converting to a JSON 
string, as we would need other similar methods for other representations such 
as XML.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [nifi] mattyb149 commented on a change in pull request #3953: NIFI-5901 Added JSON/JSONB support to PutDatabaseRecord

2020-01-06 Thread GitBox
mattyb149 commented on a change in pull request #3953: NIFI-5901 Added 
JSON/JSONB support to PutDatabaseRecord
URL: https://github.com/apache/nifi/pull/3953#discussion_r363446188
 
 

 ##
 File path: 
nifi-nar-bundles/nifi-extension-utils/nifi-processor-utils/src/main/java/org/apache/nifi/processor/util/pattern/Put.java
 ##
 @@ -89,7 +89,7 @@ public void onTrigger(ProcessContext context, ProcessSession 
session, FC functio
 session.transfer(routedFlowFiles, relationship)));
 
 if (flowFiles == null || flowFiles.isEmpty()) {
-logger.debug("No incoming FlowFiles.");
+//logger.debug("No incoming FlowFiles.");
 
 Review comment:
   This should probably be restored as it appears to be useful for debugging 
live flows


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [nifi] mattyb149 commented on a change in pull request #3953: NIFI-5901 Added JSON/JSONB support to PutDatabaseRecord

2020-01-06 Thread GitBox
mattyb149 commented on a change in pull request #3953: NIFI-5901 Added 
JSON/JSONB support to PutDatabaseRecord
URL: https://github.com/apache/nifi/pull/3953#discussion_r363445186
 
 

 ##
 File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutDatabaseRecord.java
 ##
 @@ -252,6 +258,16 @@
 .defaultValue("false")
 .build();
 
+static final PropertyDescriptor MAP_RECORD_TO_JSON = new 
PropertyDescriptor.Builder()
+.name("put-db-record-map-record-to-json")
+.displayName("Map \"record\" types to JSON")
 
 Review comment:
   What if we made this a dropdown with allowable values vs booleans? It 
appears that setting the JDBC type to `OTHER` supports both JSON and JSONB (at 
least for PostgreSQL), but in the future we may want to support the SQLXML type 
for inserting into XML columns and so on. The description of the `JSON` option 
should probably explicitly say that the JDBC type is set to `OTHER`, as that 
might be helpful information for other databases that do things differently.


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:
us...@infra.apache.org


With regards,
Apache Git Services