Tartarus0zm commented on code in PR #1620:
URL: https://github.com/apache/auron/pull/1620#discussion_r2516519413
##########
spark-extension/src/main/java/org/apache/auron/spark/configuration/SparkAuronConfiguration.java:
##########
@@ -114,14 +113,7 @@ public class SparkAuronConfiguration extends
AuronConfiguration {
"auron.partialAggSkipping.minRows")
.description("minimum number of rows to trigger partial aggregate
skipping.")
.intType()
- .defaultValue(
Review Comment:
> removed this because it causes NPE in `SparkAuronConfigurationTest`, the
UT doesn't initialize `SparkEnv`, and `AuronAdaptor.getAuronConfiguration()`
depends on it to get SparkConf, triggering NPE.
>
> ```
> public AuronConfiguration getAuronConfiguration() {
> return new SparkAuronConfiguration(SparkEnv$.MODULE$.get().conf()); //
NPE in UT
> }
> ```
>
> I'll try to fix the UT first. Do you have any better suggestions?
@yew1eb Thanks for your feedback.
It appears our current configuration design has some shortcomings.
Typically, a configuration item's default value should not depend on another
configuration item. Such cases are usually resolved through code. This is a
legacy issue. I will resolve this issue in #1621 as soon as possible.
--
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]
