abdullah alamoudi has posted comments on this change.

Change subject: ASTERIXDB-1302 ASTERIXDB-1301 Fix Socket Feed Connection
......................................................................


Patch Set 2:

(8 comments)

https://asterix-gerrit.ics.uci.edu/#/c/660/2/asterix-app/src/main/java/org/apache/asterix/api/common/AsterixHyracksIntegrationUtil.java
File 
asterix-app/src/main/java/org/apache/asterix/api/common/AsterixHyracksIntegrationUtil.java:

Line 90:             System.err.println("Using the path: " + tempPath);
> Remove again?
I'd like to keep this if it is not a big deal.
This prints the working dir of the test cluster which is useful if someone 
wants to see what is happening in storage.


https://asterix-gerrit.ics.uci.edu/#/c/660/2/asterix-app/src/main/java/org/apache/asterix/aql/translator/QueryTranslator.java
File 
asterix-app/src/main/java/org/apache/asterix/aql/translator/QueryTranslator.java:

Line 619:             // #. initialize DatasetIdFactory if it is not 
initialized.
> I'm a little confused about all these changes to comments. Are they done by
Yes. they are done by the formatter. I actually enabled comment formatting just 
so it breaks comments into multiple lines if it gets too long but it seems like 
it always adds a space after the beginning of the comment.

I have disabled it and removed the spaces after //


https://asterix-gerrit.ics.uci.edu/#/c/660/2/asterix-app/src/test/java/org/apache/asterix/test/runtime/ExecutionTestUtil.java
File 
asterix-app/src/test/java/org/apache/asterix/test/runtime/ExecutionTestUtil.java:

Line 62:         // HDFSCluster.getInstance().setup();
> We don't need that anymore?
Of course we do. I just disable it when I try to debug because it takes a long 
time to boot on my machine.

I have added it back.


https://asterix-gerrit.ics.uci.edu/#/c/660/2/asterix-app/src/test/resources/runtimets/queries/feeds/drop-dataverse-with-disconnected-feed/drop-dataverse-with-disconnected-feed.4.update.aql
File 
asterix-app/src/test/resources/runtimets/queries/feeds/drop-dataverse-with-disconnected-feed/drop-dataverse-with-disconnected-feed.4.update.aql:

Line 20:  * Description  : Drop a dataverse with disconnected feed 
> WS
Done


https://asterix-gerrit.ics.uci.edu/#/c/660/2/asterix-app/src/test/resources/runtimets/queries/feeds/drop-dataverse-with-disconnected-feed/drop-dataverse-with-disconnected-feed.5.ddl.aql
File 
asterix-app/src/test/resources/runtimets/queries/feeds/drop-dataverse-with-disconnected-feed/drop-dataverse-with-disconnected-feed.5.ddl.aql:

Line 20:  * Description  : Drop a dataverse with disconnected feed 
> WS
Done


https://asterix-gerrit.ics.uci.edu/#/c/660/2/asterix-common/src/test/java/org/apache/asterix/test/server/OpenSocketFileTestServer.java
File 
asterix-common/src/test/java/org/apache/asterix/test/server/OpenSocketFileTestServer.java:

Line 80:             } catch (Throwable th) {
> Ok, this is only test code.  But maybe we should at least log what's happen
Done


https://asterix-gerrit.ics.uci.edu/#/c/660/2/asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/SocketInputStream.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/SocketInputStream.java:

Line 36:     private boolean closed;
> What is the meaning of this? Could you find a more expressive name?
Done


Line 77:         if (closed) {
> Why is this the only "read" method that cares about "closed"?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f6e982440d3577343f2479c3779653a9c3db614
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <[email protected]>
Gerrit-Reviewer: Ildar Absalyamov <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to