parthchandra commented on PR #4366:
URL: 
https://github.com/apache/datafusion-comet/pull/4366#issuecomment-5320446796

   Some follow up items from A.2 (#4952) -
   
     **1. CI doesn't actually compile the Delta Scala until A.6a.** CI never 
runs a `-Pcontrib-delta` Scala compile. Right now that's checked by hand); 
between A.2 and A.6a, if someone changes `CometScanContrib` or
     `CometContribScanMarker` in core, they could break the Delta Scala and CI 
wouldn't catch it.
   
     **2. `DeltaConf` shows up before the things it controls exist.** A.2 adds 
`DeltaConf` (the table lists it under A.4a), and its settings only start doing 
anything in A.3–A.5:
   
     - `spark.comet.scan.deltaNative.enabled` defaults to `true`.  It doesn't 
do anything today, but  once A.3b/A.4b are done  it'll turn a native path on by 
default while it's still experimental. I'd default it to `false` until the 
feature is solid.
     - `spark.comet.delta.cdf.maxPartitions` is already here even though CDF 
isn't until A.5, and it's under a different prefix (`spark.comet.delta.cdf.*`) 
than the other three (`spark.comet.scan.deltaNative.*`). Let's settle on one 
prefix, and maybe hold the CDF setting back to A.5.
     - `dataFileConcurrencyLimit` says "2 to 8 is typical" in its docs but 
defaults to `1` and only checks that it's positive.
   
     **3. The delta-spark version pin isn't tracked anywhere.** A.2's 
description mentions the delta-spark 4.1.1 pin is a decision we're putting off, 
but there's no issue for it, and the gate script already hard-codes spark-4.1 → 
delta-spark 4.1.x. 
   
     **4. These configs won't make it into the generated docs.** `DeltaConf` 
lives outside `CometConf`, so `GenerateDocs` won't see them. 
     **5. (Optional) noticing when two contribs grab the same scan.** The 
"first claim wins" rule is just documented, not enforced. If we want core to 
actually warn about a misconfigured setup, the debug-only "ask everyone and log 
if more than one claims" idea from the A.2 thread would do it.
   


-- 
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]

Reply via email to