Copilot commented on code in PR #13149:
URL: https://github.com/apache/gluten/pull/13149#discussion_r4110749760
##########
gluten-core/src/main/scala/org/apache/gluten/extension/GlutenSessionExtensions.scala:
##########
@@ -43,8 +43,15 @@ private[gluten] class GlutenSessionExtensions
injector.control.disableOn {
session =>
val glutenEnabledForThread =
-
Option(session.sparkContext.getLocalProperty(GLUTEN_ENABLE_FOR_THREAD_KEY))
- .forall(_.toBoolean)
+
Option(session.sparkContext.getLocalProperty(GLUTEN_ENABLE_FOR_THREAD_KEY)).forall
{
+ value =>
+ try value.toBoolean
+ catch {
+ case _: IllegalArgumentException =>
+ throw new IllegalArgumentException(
+ s"$GLUTEN_ENABLE_FOR_THREAD_KEY should be boolean, but was
$value")
Review Comment:
This adds a new externally observable error-message path, but the existing
thread-level suite only exercises valid `false` and unset values and does not
assert the invalid-value exception or its message. Please add a regression test
that sets this local property to a value such as `yes`, triggers the extension
rule, and verifies the exception names `gluten.enabledForCurrentThread` and the
offending value so this contract cannot regress.
--
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]
