epugh commented on code in PR #4715:
URL: https://github.com/apache/solr/pull/4715#discussion_r3739707994
##########
solr/solrj/src/test/org/apache/solr/common/util/EnvUtilsTest.java:
##########
@@ -143,4 +145,54 @@ public void testFlippingDisabledToEnabledPropertyName() {
EnvUtils.init(false, env, defaultProps);
assertEquals(false, EnvUtils.getPropertyAsBool("solr.ui.enabled"));
}
+
+ /**
+ * These env vars must map directly to their current sysprop name, not to a
legacy/intermediate
+ * name that DeprecatedSystemPropertyMappings.properties also treats as
deprecated -- otherwise
+ * EnvUtils' own deprecation-forwarding logic trips on itself and logs a
confusing warning, even
+ * though the value still resolves correctly via that indirection. A
value-only assertion wouldn't
+ * catch a regression here, since the value resolves fine either way -- the
warning is the actual
+ * symptom, so this asserts on both.
+ *
+ * <p>SOLR_ALWAYS_ON_TRACE_ID is the same pattern (see {@link
#getPropWithCamelCase}) but is
+ * deliberately excluded here: it shares a target sysprop with that other
test, and this test
+ * would clobber it with a non-boolean value depending on random test
execution order.
+ *
+ * <p>The LogListener is scoped to only these six properties' names (rather
than listening for
Review Comment:
I was on the fence about this test, both all the comments and the use of
loglisterner, but since this bug was "already there" and didn't get caught, I
liked having this very much more rigourse test.
##########
solr/solrj/src/resources/EnvToSyspropMappings.properties:
##########
@@ -1,17 +1,18 @@
# Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements; and to You under the Apache License, Version
2.0.
#
-# Mapping from Environment variable to system property
-# This file only contains non-standard mappings that do not follow the
standard naming convention
-# Map to nothing to avoid setting any system property for the env.variable
-# CamelCase properties are mapped to dot separated lowercase
-# This way, env SOLR_FOO_BAR will also match property 'solr.foo.bar' without a
mapping in this file
-# TODO: Deprecate non-standard sysprops and standardize on solr.foo.bar in
Solr 10
+# Mapping from Environment variable to system property.
+# This file only contains non-standard mappings that do not follow the
standard naming convention.
+# Map to nothing to avoid setting any system property for the env.variable.
+# Env var names are mapped to sys props by lowercasing and replacing '_' with
'.', so
+# SOLR_FOO_BAR will match property 'solr.foo.bar' without needing a mapping in
this file.
+# Separately, sysprop lookups (EnvUtils.getProperty) also fall back from
camelCase to
+# dot-separated form (e.g. 'solr.logLevel' <-> 'solr.log.level'), independent
of this file.
+# TODO: Deprecate non-standard sysprops and standardize on solr.foo.bar.
AWS_PROFILE=aws.profile
-SOLR_ALWAYS_ON_TRACE_ID=solr.alwaysOnTraceId
-SOLR_AUTH_JWT_ALLOW_OUTBOUND_HTTP=solr.auth.jwt.allowOutboundHttp
-SOLR_HIDDEN_SYS_PROPS=solr.hiddenSysProps
+SOLR_ALWAYS_ON_TRACE_ID=solr.tracing.always.on.enabled
Review Comment:
turns out the pattern that avoids "double hops" is to use the permanant
property name. not the old name that then needs another hop of conversion!
##########
solr/solrj/src/resources/EnvToSyspropMappings.properties:
##########
@@ -1,17 +1,18 @@
# Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements; and to You under the Apache License, Version
2.0.
#
-# Mapping from Environment variable to system property
-# This file only contains non-standard mappings that do not follow the
standard naming convention
-# Map to nothing to avoid setting any system property for the env.variable
-# CamelCase properties are mapped to dot separated lowercase
-# This way, env SOLR_FOO_BAR will also match property 'solr.foo.bar' without a
mapping in this file
-# TODO: Deprecate non-standard sysprops and standardize on solr.foo.bar in
Solr 10
+# Mapping from Environment variable to system property.
Review Comment:
redoing this comment to make it clear hopefully for the future
--
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]
