-
Notifications
You must be signed in to change notification settings - Fork 28.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-45912][SQL] Enhancement of XSDToSchema API: Change to HDFS API for cloud storage accessibility #43789
Conversation
} catch { | ||
case _: Throwable => | ||
// Handle case where it was added with sc.addFile | ||
val addFileUrl = SparkFiles.get(xsdPath.toString) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When they are added by different scheme, e.g., hdfs://my_xsd_file
, they are downloaded into local so SparkFiles.get(xsdPath.toString)
should always the local file path. so you won't need to call hadoop FS.
val fs = xsdPath.getFileSystem(SparkHadoopUtil.get.conf) | ||
fs.open(xsdPath) | ||
} catch { | ||
case _: Throwable => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should probably rethrow the exception if that's not found in SparkFiles
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g.,
} catch {
case e: Throwable =>
logInfo(s"$xsdPath was not found, falling back to look up files added by Spark")
val f = new File(SparkFiles.get(xsdPath.toString))
if (f.exists()( {
new FileInputStream(f)
} else {
throw e
}
Not tested :-).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise
Merged to master. |
What changes were proposed in this pull request?
Previously, it utilized
java.nio.path
, which limited file reading to local file systems only. By changing this to an HDFS-compatible API, we now enable the XSDToSchema function to access files in cloud storage.Why are the changes needed?
We want to enable the XSDToSchema function to access files in cloud storage.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Unit tests
Was this patch authored or co-authored using generative AI tooling?
No