Till Westmann has posted comments on this change. Change subject: Updated to match code changes to asterix ......................................................................
Patch Set 15: (19 comments) A first round of comments. https://asterix-gerrit.ics.uci.edu/#/c/1227/15/.gitignore File .gitignore: Line 3: asteri-opt-bom/target Shouldn't those be covered by "target" below? Line 12: *.hprof Are these actually created by the build? https://asterix-gerrit.ics.uci.edu/#/c/1227/15//COMMIT_MSG Commit Message: Line 7: Updated to match code changes to asterix Would be nice to have a better description here. https://asterix-gerrit.ics.uci.edu/#/c/1227/15/asterix-bad/src/main/java/org/apache/asterix/bad/ChannelJobService.java File asterix-bad/src/main/java/org/apache/asterix/bad/ChannelJobService.java: Line 41: import org.json.JSONException; We shouldn't use org.json anymore - the license is category X now. Line 60: e.printStackTrace(); Could we at least log this with a log4j logger? Line 113: //TODO: Allow Repetitive Channels to use YMD durations WS Line 125: empty line Line 137: empty line Line 163: } catch (Exception e) { Remove the try-catch? Line 167: throw new Exception(); Give an error message? Line 186: System.out.println(response.toString()); Log to a logger? Line 189: throw new Exception(); Give an error message? Line 193: LOGGER.info("Broker connection failed to write"); Pass the exception it? https://asterix-gerrit.ics.uci.edu/#/c/1227/15/asterix-bad/src/main/java/org/apache/asterix/bad/metadata/Procedure.java File asterix-bad/src/main/java/org/apache/asterix/bad/metadata/Procedure.java: Line 96: empty lines? https://asterix-gerrit.ics.uci.edu/#/c/1227/15/asterix-bad/src/main/java/org/apache/asterix/bad/runtime/RepetitiveChannelOperatorDescriptor.java File asterix-bad/src/main/java/org/apache/asterix/bad/runtime/RepetitiveChannelOperatorDescriptor.java: Line 65: } catch (Exception e) { Can't we just throw HyracksDataException from the beginning? https://asterix-gerrit.ics.uci.edu/#/c/1227/15/asterix-bad/src/main/java/org/apache/asterix/bad/runtime/RepetitiveChannelOperatorNodePushable.java File asterix-bad/src/main/java/org/apache/asterix/bad/runtime/RepetitiveChannelOperatorNodePushable.java: Line 62: Huh? https://asterix-gerrit.ics.uci.edu/#/c/1227/15/asterix-bad/src/main/resources/lang-extension/lang.txt File asterix-bad/src/main/resources/lang-extension/lang.txt: Line 59: | "broker" pairId = QualifiedName() ifExists = IfExists() WS https://asterix-gerrit.ics.uci.edu/#/c/1227/15/asterix-bad/src/test/resources/runtimets/queries/channel/drop_channel_check_metadata/drop_channel_check_metadata.3.query.aql File asterix-bad/src/test/resources/runtimets/queries/channel/drop_channel_check_metadata/drop_channel_check_metadata.3.query.aql: Line 3: for $result in dataset Metadata.Channel WS https://asterix-gerrit.ics.uci.edu/#/c/1227/15/asterix-opt-bom/pom.xml File asterix-opt-bom/pom.xml: Line 20: xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd";> Formatting - WS and indentation. -- To view, visit https://asterix-gerrit.ics.uci.edu/1227 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I010b81776543e127f09f046a8601bb7184f7de9a Gerrit-PatchSet: 15 Gerrit-Project: asterixdb-bad Gerrit-Branch: master Gerrit-Owner: Steven Jacobs <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Steven Jacobs <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
