Steven Jacobs has posted comments on this change. Change subject: Updated to match code changes to asterix ......................................................................
Patch Set 15: (19 comments) Addressed Comments. Uploading new patchset 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? Done Line 12: *.hprof > Are these actually created by the build? build is for sure. I removed the .hprof for now because I can't remember for sure. 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. Done 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. Done Line 60: e.printStackTrace(); > Could we at least log this with a log4j logger? Done Line 113: //TODO: Allow Repetitive Channels to use YMD durations > WS Done Line 125: > empty line Done Line 137: > empty line Done Line 163: } catch (Exception e) { > Remove the try-catch? Done Line 167: throw new Exception(); > Give an error message? Done Line 186: System.out.println(response.toString()); > Log to a logger? Done Line 189: throw new Exception(); > Give an error message? Done Line 193: LOGGER.info("Broker connection failed to write"); > Pass the exception it? Done 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? Done 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? The issue here is that HyracksConnection throws a generic exception. I went ahead and push this down into the nodepushable though. 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? This is actually the intended use of the new ActiveSourceOperatorNodePushable Basically the start() method runs until an external source kills the running executor service. 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 Done 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 Done 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. Done -- 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
