>From Hussain Towaileb <[email protected]>:

Attention is currently required from: Murtadha Hubail, Wail Alkowaileet.
Hussain Towaileb 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 7: Code-Review+1

(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/84c16ffd_f926889c
PS6, Line 76: NO_OP_FILTER
> You have this defined in the Utils too. You should just move it to IoUtil.
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/41e5dcdc_88b83311
PS6, Line 111: HyracksDataException
> This can't be thrown right now
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/2e5abb48_7c616056
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
Done


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/25c8a73d_838e3017
PS6, Line 46:     public CloudResettableInputStream getS3Stream() {
> The name isn't cloud agnostic. Same thing appears in other classes that are 
> supposed to be agnostic.
Done


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/19913b78_a17d5a7d
PS6, Line 23: Utils
> I don't think we need this class just yet since nothing in here is really 
> cloud related. […]
Done


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/97688b25_ceb87d92
PS6, Line 91: Thread.currentThread().interrupt();
> you should throw ex after this
Done


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/17dffa29_64890f6f
PS6, Line 65: final static
> static final. […]
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/905c5d1e_369da82d
PS6, Line 90: do better exception handling
> indeed :)
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/68f187d4_29170ce1
PS6, Line 162: assert buffer.remaining() == 0;
> assertion only work while we are running some of our tests. […]
Done


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/6e285a2b_9b11bd4f
PS6, Line 30: public
> The recommended practice is to make the constructor for such util classes 
> private and put throw new  […]
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/3014f24c_cf477606
PS6, Line 50: !listObjectsResponse.isTruncated()
> Boolean.FALSE.equals(listObjectsResponse. […]
Done



--
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: 7
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: Murtadha Hubail <[email protected]>
Gerrit-Attention: Wail Alkowaileet
Gerrit-Comment-Date: Thu, 30 Mar 2023 17:53:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Murtadha Hubail <[email protected]>
Gerrit-MessageType: comment

Reply via email to