dsmiley commented on a change in pull request #37:
URL: https://github.com/apache/solr/pull/37#discussion_r598722708
##########
File path:
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java
##########
@@ -290,70 +275,83 @@ protected void handleInitializationException(Exception
exception, Initialization
/**
* Handles an exception that occurred while loading the configuration
resource.
+ * The default implementation will return {@link #cacheElevationProvider} if
present, while
+ * also logging the error. If that is null (e.g. on startup) then the
exception is thrown.
+ * When re-throwing, wrap in a {@link SolrException}.
*
- * @param e The exception caught.
- * @param resourceAccessIssue <code>true</code> if the exception has been
thrown
- * because the resource could not be accessed
(missing or cannot be read)
- * or the config file is empty;
<code>false</code> if the resource has
- * been found and accessed but the error occurred
while loading the resource
- * (invalid format, incomplete or corrupted).
- * @return The {@link ElevationProvider} to use if the exception is
absorbed. If {@code null}
- * is returned, the {@link #NO_OP_ELEVATION_PROVIDER} is used but
not cached in
- * the {@link ElevationProvider} cache.
- * @throws E If the exception is not absorbed.
+ * @param e The exception caught. It will extend {@link IOException} if
there was a resource
+ * access issue.
+ * @return The {@link ElevationProvider} to use if the exception is absorbed
(vs re-thrown).
*/
- protected <E extends Exception> ElevationProvider
handleConfigLoadingException(E e, boolean resourceAccessIssue) throws E {
- throw e;
+ protected <E extends Exception> ElevationProvider
handleConfigLoadingException(E e) {
+ if (cacheElevationProvider != null) { // thus at runtime (a search is
in-progress)
+ String msg = e.toString(); // declare to avoid log isEnabled check
Review comment:
See `gradle/validation/validate-log-calls.gradle` Erick Erickson
insisted we guard many of our log calls with a log enabled check. LUCENE-7788
IMO there shouldn't be checks on an error level but Erick had stronger
opinions than I did at the time :-)
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
