pingtimeout commented on code in PR #1532:
URL: https://github.com/apache/polaris/pull/1532#discussion_r2081340225
##########
quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java:
##########
@@ -57,26 +64,53 @@ public class ProductionReadinessChecks {
*/
private static final String WARNING_SIGN_UTF_8 = "\u0000\u26A0\uFE0F";
+ private static final String SEVERE_SIGN_UTF_8 = "\u0000\uD83D\uDED1";
+
/** A simple warning sign displayed when the character set is not UTF-8. */
private static final String WARNING_SIGN_PLAIN = "!!!";
+ private static final String SEVERE_SIGN_PLAIN = "***STOP***";
+
public void warnOnFailedChecks(
- @Observes Startup event, Instance<ProductionReadinessCheck> checks) {
+ @Observes Startup event,
+ Instance<ProductionReadinessCheck> checks,
+ QuarkusReadinessConfiguration config) {
List<Error> errors = checks.stream().flatMap(check ->
check.getErrors().stream()).toList();
if (!errors.isEmpty()) {
- String warning =
- Charset.defaultCharset().equals(StandardCharsets.UTF_8)
- ? WARNING_SIGN_UTF_8
- : WARNING_SIGN_PLAIN;
- LOGGER.warn("{} Production readiness checks failed! Check the warnings
below.", warning);
+ var utf8 = Charset.defaultCharset().equals(StandardCharsets.UTF_8);
+ var warning = utf8 ? WARNING_SIGN_UTF_8 : WARNING_SIGN_PLAIN;
+ var severe = utf8 ? SEVERE_SIGN_UTF_8 : SEVERE_SIGN_PLAIN;
+ var hasSevere = errors.stream().anyMatch(Error::severe);
+ LOGGER
+ .makeLoggingEventBuilder(hasSevere ? Level.ERROR : Level.WARN)
+ .log(
+ "{} Production readiness checks failed! Check the warnings
below.",
+ hasSevere ? severe : warning);
errors.forEach(
error ->
- LOGGER.warn(
- "- {} Offending configuration option: '{}'.",
- error.message(),
- error.offendingProperty()));
- LOGGER.warn(
- "Refer to
https://polaris.apache.org/in-dev/unreleased/configuring-polaris-for-production
for more information.");
+ LOGGER
+ .makeLoggingEventBuilder(error.severe() ? Level.ERROR :
Level.WARN)
+ .log(
+ "- {} {} Offending configuration option: '{}'.",
+ error.severe() ? severe : warning,
+ error.message(),
+ error.offendingProperty()));
+ LOGGER
+ .makeLoggingEventBuilder(hasSevere ? Level.ERROR : Level.WARN)
+ .log(
+ "Refer to
https://polaris.apache.org/in-dev/unreleased/configuring-polaris-for-production
for more information.");
+
+ if (hasSevere) {
+ if (!config.ignoreSecurityIssues()) {
+ throw new IllegalStateException(
Review Comment:
nit: the current method is named `warnOnFailedChecks`. The name does not
suggest that an ISE can be thrown, and there is no Javadoc that tells this
either. It could be preferable to have two methods:
* `warnOnFailedChecks`
* `maybeAbortStartupOnSecurityIssues`
Maybe add a `TODO` comment so that this becomes an LHF for new contributors?
--
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: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
