asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Till Westmann (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb-bad[master]: Updated to match code changes to asterix
Date Mon, 05 Dec 2016 05:36:04 GMT
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 <sjaco002@ucr.edu>
Gerrit-Reviewer: Ian Maxon <imaxon@apache.org>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Steven Jacobs <sjaco002@ucr.edu>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message