Copilot commented on code in PR #11458:
URL: https://github.com/apache/gravitino/pull/11458#discussion_r3361987066
##########
conf/gravitino-env.sh.template:
##########
@@ -33,6 +33,7 @@ GRAVITINO_VERSION=GRAVITINO_VERSION_PLACEHOLDER
# # Appended to JAVA_OPTS by launch scripts; set
GRAVITINO_MEM to change heap/metaspace sizes.
# UI selection behaviour:
-# - By default this template does not set `GRAVITINO_USE_WEB_V2`, so the
container or environment can control which UI is used.
-# - To force the v1 UI, explicitly set it to `false`; to enable the v2 UI, set
it to `true` (for example: `export GRAVITINO_USE_WEB_V2=true`).
-# GRAVITINO_USE_WEB_V2=false
\ No newline at end of file
+# - Default to the v2 UI when GRAVITINO_USE_WEB_V2 is unset.
+# - If GRAVITINO_USE_WEB_V2 is already provided by container/k8s env, keep
that value.
+# - To force the v1 UI, set GRAVITINO_USE_WEB_V2=false.
+export GRAVITINO_USE_WEB_V2=${GRAVITINO_USE_WEB_V2:-true}
Review Comment:
The comment says the default applies only when GRAVITINO_USE_WEB_V2 is
"unset", and that a container/k8s-provided value is kept. However, the
`${VAR:-default}` expansion also treats an explicitly-empty value as absent and
replaces it with `true`, so an empty-but-set env var will not be kept. Also,
quoting the expansion avoids any unintended word-splitting.
##########
conf/gravitino-env.sh.template:
##########
@@ -33,6 +33,7 @@ GRAVITINO_VERSION=GRAVITINO_VERSION_PLACEHOLDER
# # Appended to JAVA_OPTS by launch scripts; set
GRAVITINO_MEM to change heap/metaspace sizes.
# UI selection behaviour:
-# - By default this template does not set `GRAVITINO_USE_WEB_V2`, so the
container or environment can control which UI is used.
-# - To force the v1 UI, explicitly set it to `false`; to enable the v2 UI, set
it to `true` (for example: `export GRAVITINO_USE_WEB_V2=true`).
-# GRAVITINO_USE_WEB_V2=false
\ No newline at end of file
+# - Default to the v2 UI when GRAVITINO_USE_WEB_V2 is unset.
+# - If GRAVITINO_USE_WEB_V2 is already provided by container/k8s env, keep
that value.
+# - To force the v1 UI, set GRAVITINO_USE_WEB_V2=false.
+export GRAVITINO_USE_WEB_V2=${GRAVITINO_USE_WEB_V2:-true}
Review Comment:
This change flips the default UI selection to v2, but some user-facing docs
still describe the old behavior (e.g., README.md says the template defaults to
`false` and docs/webui-v2.md says the template does not set
GRAVITINO_USE_WEB_V2). These should be updated to avoid misleading users about
the new default and how to revert to v1.
--
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]
