Call globStatus directly via PY4J in hdfs_glob to avoid calling hadoop command #10599
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Yinqing Hao <haoyinqing@gmail.com>
yinqingh
requested review from
jlowe,
gerashegalov,
GaryShen2008,
NvTimLiu,
YanxuanLiu and
sameerz
|
This is also a follow up of 9dc04aa#r1477618915 @gerashegalov , thanks! |
|
build |
jlowe
reviewed
Mar 15, 2024
| # status.getPath().toString() return string like "hdfs://hostname:8020/src/test/resources/parquet-testing/data/single_nan.parquet" | ||
| # but pathlib.Path will remove the first "/" and convert it to "hdfs:/hostname:8020/src/test/resources/parquet-testing/data/single_nan.parquet" and then this path becomes illegal. | ||
| # so we need to process the path like this. | ||
| p = f'hdfs:{status.getPath().toUri().getPath()}' |
There was a problem hiding this comment.
you can get the scheme "hdfs" from the URI instead of hardcoding it.
There was a problem hiding this comment.
Removed the hardcoding for scheme "hdfs". Thanks!
Signed-off-by: Yinqing Hao <haoyinqing@gmail.com>
Signed-off-by: Yinqing Hao <haoyinqing@gmail.com>
Signed-off-by: Yinqing Hao <haoyinqing@gmail.com>
|
The github action build failures will be fixed in #10608 |
Signed-off-by: Yinqing Hao <haoyinqing@gmail.com>
|
build |
jlowe
approved these changes
Mar 18, 2024
gerashegalov
approved these changes
Mar 18, 2024
| sc = get_spark_i_know_what_i_am_doing().sparkContext | ||
| config = sc._jsc.hadoopConfiguration() | ||
| fs_path = sc._jvm.org.apache.hadoop.fs.Path(full_pattern) | ||
| fs = sc._jvm.org.apache.hadoop.fs.FileSystem.get(fs_path.toUri(), config) |
There was a problem hiding this comment.
nit. no need to fix. a handy idiom especially if you need to access the fs object only once with the added benefit avoiding accessing sc._jvm
Suggested change
| fs = sc._jvm.org.apache.hadoop.fs.FileSystem.get(fs_path.toUri(), config) | |
| fs = fs_path.getFileSystem(config) |
There was a problem hiding this comment.
This is helpful. Thanks!
| :return: generator of matched files | ||
| """ | ||
| path_str = path.as_posix() | ||
| if not path_str.startswith('hdfs:'): |
There was a problem hiding this comment.
not for this PR, but this should be agnostic of a particular non-local FileSystem path.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To fix #10586
In rapids_it-EGX-Yarn job, the integration tests run in container which doesn't have hadoop cli installed. To fix this, I update
hdfs_globfunction to call globStatus directly via PY4J to avoid callinghadoopcommand