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 <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Jianfeng Jia <[email protected]>
Gerrit-HasComments: Yes

Reply via email to