andygrove commented on code in PR #6838:
URL: https://github.com/apache/datafusion-comet/pull/6838#discussion_r4260613317
##########
spark/src/main/scala/org/apache/comet/CometConf.scala:
##########
@@ -1286,13 +1286,17 @@ object CometConf extends ShimCometConf {
val COMET_LIBHDFS_SCHEMES_KEY = "fs.comet.libhdfs.schemes"
+ // Declared so the value appears in the generated config reference and can
be set via
+ // `spark.hadoop.*`, but never read through this entry:
`NativeConfig.resolveLibhdfsSchemes`
Review Comment:
This says the entry is never read, but `DeltaScanSupport.libhdfsSchemes` in
`contrib/delta-spark` still calls `COMET_LIBHDFS_SCHEMES.get()`. That came in
with #5365 after this PR was opened. `CometDeltaNativeScan` gets the list from
the Hadoop conf like the core scan does, so the Delta gate still has both cases
from #6837. A list only in `core-site.xml` makes it decline a scheme native can
read, and a runtime `SET spark.hadoop.fs.comet.libhdfs.schemes` makes it claim
a scheme native then can't open.
Could this PR move that one over too? `declineReason` already builds
`hadoopConf` a few lines up, so `val libhdfs =
NativeConfig.resolveLibhdfsSchemes(hadoopConf)` at `DeltaScanSupport.scala:297`
should do it, and `libhdfsSchemes` and its import can go. The
`DeltaScanContribSuite` test that checks it parses "exactly like core's scan
gate" would change with it, and I'll add the `run-delta-tests` label so the
Delta suites run. If you'd rather keep this PR to core, let me know and I'll
open an issue for the Delta gate. The comment here would still need to change
though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
