>From Murtadha Hubail <[email protected]>: Attention is currently required from: Hussain Towaileb, Wail Alkowaileet. Murtadha Hubail has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453 )
Change subject: [ASTERIXDB-3158][HYR]: Make IOManager support writing to cloud storage ...................................................................... Patch Set 6: (11 comments) File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/IOManager.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/bad6fd04_7da1e0b7 PS6, Line 76: NO_OP_FILTER You have this defined in the Utils too. You should just move it to IoUtil. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/363a07ee_3322241d PS6, Line 111: HyracksDataException This can't be thrown right now https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/9caac15c_63c07c1f PS6, Line 511: Returns null if this abstract pathname does not denote a directory, or if an I/O error occurs. Doesn't look like it File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/cloud/CloudFileHandle.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/2619fbcb_802238d2 PS6, Line 46: public CloudResettableInputStream getS3Stream() { The name isn't cloud agnostic. Same thing appears in other classes that are supposed to be agnostic. File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/cloud/Utils.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/6bc52dbb_d41a6076 PS6, Line 23: Utils I don't think we need this class just yet since nothing in here is really cloud related. You can move these to IoUtil. File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/cloud/clients/aws/s3/S3BufferedWriter.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/c808a0ce_503f9d9a PS6, Line 91: Thread.currentThread().interrupt(); you should throw ex after this File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/cloud/clients/aws/s3/S3CloudClient.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/8c0a8ec0_3bd9ada3 PS6, Line 65: final static static final. same for the 3 below https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/bce69f75_484d2db8 PS6, Line 90: do better exception handling indeed :) https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/87e4d5a0_b2e47a3d PS6, Line 162: assert buffer.remaining() == 0; assertion only work while we are running some of our tests. If this check is important, change it to an if statement and IllegalStateException File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/cloud/clients/aws/s3/S3Utils.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/977b0429_5ece8141 PS6, Line 30: public The recommended practice is to make the constructor for such util classes private and put throw new AssertionError("do not instantiate") inside as a hint to anyone thinking about making it public in the future. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/88e341f1_8ef18f46 PS6, Line 50: !listObjectsResponse.isTruncated() Boolean.FALSE.equals(listObjectsResponse.isTruncated() -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I362a3cbfcd3fa99f321467cb72d74d388fcdee2b Gerrit-Change-Number: 17453 Gerrit-PatchSet: 6 Gerrit-Owner: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Wail Alkowaileet Gerrit-Reviewer: Wail Alkowaileet <[email protected]> Gerrit-CC: Murtadha Hubail <[email protected]> Gerrit-Attention: Hussain Towaileb <[email protected]> Gerrit-Attention: Wail Alkowaileet Gerrit-Comment-Date: Thu, 30 Mar 2023 02:02:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
