andygrove commented on code in PR #2722:
URL: https://github.com/apache/datafusion-comet/pull/2722#discussion_r2503951107
##########
common/src/main/scala/org/apache/comet/CometConf.scala:
##########
@@ -865,6 +861,37 @@ private class TypedConfigBuilder[T](
CometConf.register(conf)
conf
}
+
+ /**
+ * Creates a [[ConfigEntry]] that has a default value, with support for
environment variable
+ * override.
+ *
+ * The value is resolved in the following priority order:
+ * 1. Spark config value (if set) 2. Environment variable value (if set)
3. Default value
+ *
+ * @param envVar
+ * The environment variable name to check for override value
+ * @param default
+ * The default value to use if neither config nor env var is set
+ * @return
+ * A ConfigEntry with environment variable support
+ */
+ def createWithEnvVarOrDefault(envVar: String, default: T): ConfigEntry[T] = {
+ val defaultValue = sys.env.get(envVar).map(converter).getOrElse(default)
Review Comment:
Thanks. I like that better. I am not sure why we use both `converter` and
`stringConverter`. Perhaps it was intended to provide extra validation. In
`createWithDefault`, we have `val transformedDefault =
converter(stringConverter(default))`, which seems redundant.
--
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]
