>From Savyasach Reddy <[email protected]>:

Attention is currently required from: Hussain Towaileb.
Savyasach Reddy has posted comments on this change. ( 
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066 )

Change subject: [ASTERIXDB-3519][EXT]: Support dynamic prefixes on HDFS
......................................................................


Patch Set 4:

(11 comments)

File 
asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestConstants.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066/comment/0b0f808f_7f6eed05
PS3, Line 37: S3_DDL_TEMPLATE_DEFAULT
> S3_TEMPLATE_DEFAULT_NO_PARANTHESES_WITH_COLONS
Done


File 
asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066/comment/4fc90e64_41d90a36
PS3, Line 2567: else
> Same as above
Done


File 
asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/input/HDFSDataSourceFactory.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066/comment/e0ab026b_62f59f1e
PS3, Line 152: LOGGER.warn("The provided external dataset configuration 
returned no files from the external source");
> Use warning collector to warn, see examples in S3Utils.
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066/comment/392c2c73_98aa957f
PS3, Line 153: hdfsConf
> Let's have a method to do this in the HDFS utils, and here instead set the 
> KEY_PATH to empty string.
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066/comment/cc3da553_454aa950
PS3, Line 153: (ExternalDataConstants.KEY_HADOOP_INPUT_DIR, "");
> Let's see what happens if we pass "" vs "/". Will it return everything or 
> empty, .. […]
hdfsConf.set(ExternalDataConstants.KEY_HADOOP_INPUT_DIR, "/") is returning all 
the files.
hdfsConf.set(ExternalDataConstants.KEY_HADOOP_INPUT_DIR, "") doesn't return any 
files due to the function HDFSDataSourceFactory#getInputSplits


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066/comment/c916c817_eef09793
PS3, Line 158: hdfsConf.set(ExternalDataConstants.KEY_HADOOP_INPUT_DIR, "");
> Throw external source error exception.
I've updated with the same.
But for S3, if the prefix doesn't match, we aren't throwing an error, just a 
warning. Shouldn't we do the same over here instead of an error?


File 
asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataPrefix.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066/comment/2fbee68b_837fcece
PS3, Line 80: 
Objects.equals(configuration.get(ExternalDataConstants.KEY_EXTERNAL_SOURCE_TYPE),
            :                         ExternalDataConstants.KEY_HDFS_URL)
> Let's have a function for this in HDFSUtils. […]
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066/comment/49e72340_0ce960cd
PS3, Line 81: KEY_HDFS_URL
> Compare it with KEY_ADAPTER_NAME_HDFS, use . […]
Done


File 
asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/HDFSUtils.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066/comment/af74494d_cfdd1d8a
PS3, Line 236:
> Add this comment: […]
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066/comment/73a921f6_1e2b4b2a
PS3, Line 404: updateRoot
> updateRootPath
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066/comment/b85670ad_1e3f0a8b
PS3, Line 577:         try {
> Add a comment here that this is for validation purpose for external data 
> prefix stuff.
Done



--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19066
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: I1bdbcd44c059f64f2da436a40ac3f59293442cf2
Gerrit-Change-Number: 19066
Gerrit-PatchSet: 4
Gerrit-Owner: Savyasach Reddy <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-CC: Hussain Towaileb <[email protected]>
Gerrit-Attention: Hussain Towaileb <[email protected]>
Gerrit-Comment-Date: Mon, 18 Nov 2024 13:44:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hussain Towaileb <[email protected]>
Gerrit-MessageType: comment

Reply via email to