asterixdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jianfeng Jia (Code Review)" <do-not-re...@asterix-gerrit.ics.uci.edu>
Subject Change in asterixdb[master]: This change includes the following things: 1. Allow pregelix...
Date Wed, 22 Jul 2015 06:43:56 GMT
Jianfeng Jia has posted comments on this change.

Change subject: This change includes the following things: 1. Allow pregelix statement to
cross two different dataverses. 2. Change TestsUtils to TestsExecutor which is not based on
static methods so that Pregelix can reuse its functionalities. 3. Adds ioDeviceId into th
......................................................................


Patch Set 3:

(7 comments)

What's the purpose of 
`Adds ioDeviceId into the returned result of ConnectorAPIServlet` ?

https://asterix-gerrit.ics.uci.edu/#/c/325/3/asterix-app/src/main/java/edu/uci/ics/asterix/api/common/AsterixHyracksIntegrationUtil.java
File asterix-app/src/main/java/edu/uci/ics/asterix/api/common/AsterixHyracksIntegrationUtil.java:

Line 63:         File dir = new File(System.getProperty("java.io.tmpdir") + File.separator
+ "nc1");
This "java.io.tmpdir" is used multiple times, can we make is as a static final value?


Line 65:             FileUtils.forceDelete(dir);
What kind of problem will appear if we didn't delete?


https://asterix-gerrit.ics.uci.edu/#/c/325/3/asterix-app/src/main/java/edu/uci/ics/asterix/api/http/servlet/ConnectorAPIServlet.java
File asterix-app/src/main/java/edu/uci/ics/asterix/api/http/servlet/ConnectorAPIServlet.java:

Line 111:             // Flush the cached contents of the dataset to file system.
Why do we need to flush at this time? Especially in the "doGet" method, do we update something
in the read interface?


https://asterix-gerrit.ics.uci.edu/#/c/325/3/asterix-app/src/main/java/edu/uci/ics/asterix/aql/translator/AqlTranslator.java
File asterix-app/src/main/java/edu/uci/ics/asterix/aql/translator/AqlTranslator.java:

Line 2829:             if (resultState != 1) {
why do we take the 1 as the succeed? Should it be 0 by the bash convention? Any stories? :-)


Line 2926:         pr.waitFor();
Should we catch the `pr.exitValue()` as well? In case some other system error happened cause
it exit with an non-zero value.


Line 2972:             if (s.equals("-cust-prop")) {
just curiously, what's this "-cust-prop" meaning?


https://asterix-gerrit.ics.uci.edu/#/c/325/3/asterix-app/src/test/java/edu/uci/ics/asterix/aql/translator/AqlTranslatorTest.java
File asterix-app/src/test/java/edu/uci/ics/asterix/aql/translator/AqlTranslatorTest.java:

Line 78:                 .asList(new String[] { "bin/pregelix examples/pregelix-example-0.2.13-jar-with-dependencies.jar
"
Should we make this 0.2.13 be pluggable in order to not break the test once the Pregelix get
evolved later?


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/325
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d167b64bf9ec754182b5b2fe44dfc7e5908c686
Gerrit-PatchSet: 3
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Yingyi Bu <buyingyi@gmail.com>
Gerrit-Reviewer: Ian Maxon <imaxon@apache.org>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Jianfeng Jia <jianfeng.jia@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message