diff --git a/src/main/java/io/quarkus/search/app/indexing/reporting/FailureCollector.java b/src/main/java/io/quarkus/search/app/indexing/reporting/FailureCollector.java index 778fa384..478a9dcb 100644 --- a/src/main/java/io/quarkus/search/app/indexing/reporting/FailureCollector.java +++ b/src/main/java/io/quarkus/search/app/indexing/reporting/FailureCollector.java @@ -13,7 +13,12 @@ enum Level { * Level for failures that don't prevent reindexing but cause the index to be incomplete/incorrect, * and may require maintainer's attention. */ - WARNING(Logger.Level.WARN); + WARNING(Logger.Level.WARN), + /** + * Level for failures that don't prevent reindexing but cause the index to be incomplete/incorrect, + * though maintainer's attention is not needed (the solution is just waiting). + */ + INFO(Logger.Level.INFO); public final Logger.Level logLevel; @@ -34,6 +39,14 @@ default void collect(Level level, Stage stage, String details) { void collect(Level level, Stage stage, String details, Exception exception); + default void info(Stage stage, String details) { + collect(Level.INFO, stage, details); + } + + default void info(Stage stage, String details, Exception exception) { + collect(Level.INFO, stage, details, exception); + } + default void warning(Stage stage, String details) { collect(Level.WARNING, stage, details); } diff --git a/src/main/java/io/quarkus/search/app/indexing/reporting/GithubStatusReporter.java b/src/main/java/io/quarkus/search/app/indexing/reporting/GithubStatusReporter.java index 5b44ab61..ab6f5cf7 100644 --- a/src/main/java/io/quarkus/search/app/indexing/reporting/GithubStatusReporter.java +++ b/src/main/java/io/quarkus/search/app/indexing/reporting/GithubStatusReporter.java @@ -68,6 +68,8 @@ public void report(Status status, Map> fai switch (status) { case IN_PROGRESS, SUCCESS -> { // When in progress or successful, never comment: we want to avoid unnecessary noise. + // Note this means INFO level failures will have been logged by the FailureCollector, + // but won't be reported to GitHub. } case WARNING -> { // When warning, only comment if we didn't comment the same thing recently. diff --git a/src/main/java/io/quarkus/search/app/indexing/state/IndexingState.java b/src/main/java/io/quarkus/search/app/indexing/state/IndexingState.java index a9bf76f4..d8c49559 100644 --- a/src/main/java/io/quarkus/search/app/indexing/state/IndexingState.java +++ b/src/main/java/io/quarkus/search/app/indexing/state/IndexingState.java @@ -11,11 +11,13 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; -import io.quarkus.logging.Log; import io.quarkus.search.app.indexing.reporting.Failure; import io.quarkus.search.app.indexing.reporting.FailureCollector; import io.quarkus.search.app.indexing.reporting.Status; import io.quarkus.search.app.indexing.reporting.StatusReporter; + +import io.quarkus.logging.Log; + import io.smallrye.mutiny.subscription.Cancellable; public class IndexingState { @@ -100,7 +102,7 @@ private boolean scheduleRetry() { try { scheduledRetry = retryScheduler.apply(retryConfig.delay()); // If we get here, a retry was scheduled. - warning(Stage.INDEXING, "Indexing will be tried again later."); + info(Stage.INDEXING, "Indexing will be tried again later."); return true; } catch (RuntimeException e) { // If we get here, we'll abort. @@ -115,6 +117,8 @@ private boolean scheduleRetry() { } private static Status indexingResultStatus(Map> failures) { + // INFO level is ignored for the overall status. + // We will only report INFO failures if there are other CRITICAL/WARNING failures. if (failures.get(Level.CRITICAL).isEmpty()) { if (failures.get(Level.WARNING).isEmpty()) { return Status.SUCCESS; diff --git a/src/main/java/io/quarkus/search/app/quarkusio/QuarkusIO.java b/src/main/java/io/quarkus/search/app/quarkusio/QuarkusIO.java index 7577982b..3f3185aa 100644 --- a/src/main/java/io/quarkus/search/app/quarkusio/QuarkusIO.java +++ b/src/main/java/io/quarkus/search/app/quarkusio/QuarkusIO.java @@ -7,6 +7,7 @@ import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.time.Duration; import java.time.Instant; @@ -137,7 +138,7 @@ private static void validateRepositories( // more than 7 days; so we give it 2 weeks period before we start considering that there's a sync problem: Duration duration = Duration.between(lastSync, Instant.now()); if (duration.compareTo(Duration.ofDays(14)) > 0) { - failureCollector.warning( + failureCollector.info( FailureCollector.Stage.TRANSLATION, "Localized repository '" + localized.getKey() + "' is out of sync for " + duration); } @@ -153,7 +154,7 @@ private static void validateRepositories( @Override public void close() throws IOException { for (QuarkusIOCloneDirectory directory : allSites.values()) { - directory.unprocessed().ifPresent(m -> failureCollector.warning(FailureCollector.Stage.PARSING, m)); + directory.unprocessed().ifPresent(m -> failureCollector.info(FailureCollector.Stage.PARSING, m)); } try (var closer = new Closer()) { closer.push(CloseableDirectory::close, prefetchedGuides); @@ -447,12 +448,17 @@ private Catalog translations(Repository repository, RevTree sources, String path try (InputStream file = GitUtils.file(repository, sources, path)) { return new PoParser().parseCatalog(file, false); - } catch (IOException | IllegalStateException e) { - // it may be that not all localized sites are up-to-date, in that case we just assume that the translation is not there - // and the non-translated english text will be used. + } catch (NoSuchFileException e) { + // translation may be missing, but that's just the way it is: it'll get translated when someone has time + failureCollector.info(FailureCollector.Stage.TRANSLATION, + "Unable to open translation file " + path + " : " + e.getMessage(), e); + // in the meantime we'll use non-translated English text + return new Catalog(); + } catch (IOException e) { + // opening/parsing may fail, in which case we definitely need to have a look failureCollector.warning(FailureCollector.Stage.TRANSLATION, - "Unable to parse a translation file " + path + " : " + e.getMessage(), e); - + "Unable to parse translation file " + path + " : " + e.getMessage(), e); + // in the meantime we'll use non-translated English text return new Catalog(); } } @@ -555,9 +561,9 @@ private Guide createCoreGuide(GitCloneDirectory cloneDirectory, String quarkusVe // if a file is not present we do not want to add such guide. Since if the html is not there // it means that users won't be able to open it on the site, and returning it in the search results make it pointless. - // Since this file was listed in the yaml of a rev from which the site was built the html should've been present, - // but is missing for some reason that needs investigation: - failureCollector.warning( + // Since this file was listed in the yaml of a rev from which the site was built, the html should've been present. + // So if it's missing, that's important context when investigating other errors. + failureCollector.info( FailureCollector.Stage.TRANSLATION, "Guide " + guide + " is ignored since we were not able to find an HTML content file for it."); return null; diff --git a/src/main/java/io/quarkus/search/app/util/GitUtils.java b/src/main/java/io/quarkus/search/app/util/GitUtils.java index 3cb7c463..66ecd496 100644 --- a/src/main/java/io/quarkus/search/app/util/GitUtils.java +++ b/src/main/java/io/quarkus/search/app/util/GitUtils.java @@ -3,6 +3,7 @@ import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; +import java.nio.file.NoSuchFileException; import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; @@ -59,7 +60,7 @@ public static InputStream file(Repository repo, RevTree tree, String path) throw treeWalk.setRecursive(true); treeWalk.setFilter(PathFilter.create(path)); if (!treeWalk.next()) { - throw new IllegalStateException("Missing file '%s' in '%s'".formatted(path, tree)); + throw new NoSuchFileException("Missing file '%s' in '%s'".formatted(path, tree)); } ObjectId objectId = treeWalk.getObjectId(0); ObjectLoader loader = repo.open(objectId); diff --git a/src/test/java/io/quarkus/search/app/indexing/state/IndexingStateTest.java b/src/test/java/io/quarkus/search/app/indexing/state/IndexingStateTest.java index a408b1a5..30d2a197 100644 --- a/src/test/java/io/quarkus/search/app/indexing/state/IndexingStateTest.java +++ b/src/test/java/io/quarkus/search/app/indexing/state/IndexingStateTest.java @@ -45,6 +45,9 @@ void success() { assertThat(state.isInProgress()).isTrue(); verify(statusReporterMock).report(eq(Status.IN_PROGRESS), eq(Map.of())); + // info should not affect the status + attempt.info(Stage.INDEXING, "Some info"); + // No warning/critical: success } verify(statusReporterMock).report(eq(Status.SUCCESS), anyMap()); @@ -80,6 +83,9 @@ void warning() { verify(statusReporterMock).report(eq(Status.IN_PROGRESS), eq(Map.of())); attempt.warning(Stage.INDEXING, "Some warning"); + + // info should not affect the status + attempt.info(Stage.INDEXING, "Some info"); } verify(statusReporterMock).report(eq(Status.WARNING), anyMap()); assertThat(state.isInProgress()).isFalse(); @@ -96,6 +102,9 @@ void failure_max1Attempt() { verify(statusReporterMock).report(eq(Status.IN_PROGRESS), eq(Map.of())); attempt.critical(Stage.INDEXING, "Something critical"); + + // info should not affect the status + attempt.info(Stage.INDEXING, "Some info"); } verify(statusReporterMock).report(eq(Status.CRITICAL), anyMap()); assertThat(state.isInProgress()).isFalse();