Removed ThreadConfigurableLogger class for upcoming SLF4J change #2615
Conversation
| @@ -544,7 +544,7 @@ public ScannerStream visit(ASTEQNode node, Object data) { | |||
| return ScannerStream.unindexed(node); | |||
| } | |||
| } catch (TableNotFoundException e) { | |||
| log.error(e); | |||
| log.error(String.valueOf(e)); | |||
There was a problem hiding this comment.
I think these should just be log.error(e). Is there no method for that? We want to log the full stacktrace. If we convert to a String, then we lose that.
There was a problem hiding this comment.
I see. So we need some kind of message. We might be able to get away with log.error("", e);.
| @@ -122,7 +122,7 @@ public synchronized void submit() { | |||
| range = new Range(startKey, true, endKey, literalRange.isUpperInclusive()); | |||
| } catch (IllegalArgumentException e) { | |||
| QueryException qe = new QueryException(DatawaveErrorCode.RANGE_CREATE_ERROR, e, MessageFormat.format("{0}", this.literalRange)); | |||
| log.debug(qe); | |||
| log.debug(String.valueOf(qe)); | |||
There was a problem hiding this comment.
Same thing here. We need the exception treated like a Throwable in order to see the stacktrace. Let's try log.debug("", qe);
| @@ -91,7 +91,7 @@ public static <T extends JexlNode> T expandUnfielded(ShardQueryConfiguration con | |||
| private static <T extends JexlNode> T ensureTreeNotEmpty(T script) throws EmptyUnfieldedTermExpansionException { | |||
| if (script.jjtGetNumChildren() == 0) { | |||
| NotFoundQueryException qe = new NotFoundQueryException(DatawaveErrorCode.NO_UNFIELDED_TERM_EXPANSION_MATCH); | |||
| log.warn(qe); | |||
| log.warn(String.valueOf(qe)); | |||
| @@ -1416,11 +1416,11 @@ protected ASTJexlScript timedExpandAnyFieldRegexNodes(QueryStopwatch timers, fin | |||
| } catch (EmptyUnfieldedTermExpansionException e) { | |||
| // The visitor will only throw this if we cannot expand anything resulting in empty query | |||
| NotFoundQueryException qe = new NotFoundQueryException(DatawaveErrorCode.UNFIELDED_QUERY_ZERO_MATCHES, e, MessageFormat.format("Query: ", query)); | |||
| log.info(qe); | |||
| log.info(String.valueOf(qe)); | |||
There was a problem hiding this comment.
I'm gonna stop commenting on these but just do a search for any String.valueOf and use the empty message pattern mentioned above.
| @@ -439,8 +437,8 @@ public void updateMetric(QueryMetric updatedQueryMetric, DatawavePrincipal dataw | |||
|
|
|||
| // write new entry | |||
| writeMetrics(updatedQueryMetric, Collections.singletonList(updatedQueryMetric), lastUpdated, false); | |||
| } finally { | |||
| enableLogs(true); | |||
| } catch (Exception e) { | |||
There was a problem hiding this comment.
So we can't do an empty catch here b/c that means we suppress an error. To keep this functionality the same as before, we should drop the try around the code block above.
try {
// some code
} finally {
}
Is equivalent to
// some code
| @@ -550,8 +547,8 @@ public QueryMetricListResponse query(String user, String queryId, DatawavePrinci | |||
| response.setResult(queryMetrics); | |||
|
|
|||
| response.setGeoQuery(queryMetrics.stream().anyMatch(SimpleQueryGeometryHandler::isGeoQuery)); | |||
| } finally { | |||
| enableLogs(true); | |||
| } catch (Exception e) { | |||
|
Seems like this might be good to go now once the file conflicts are resolved |
doublejai
force-pushed
the
task/GITHUB-2610
branch
from
e0e3715 to
4159dee
Compare
doublejai
force-pushed
the
task/GITHUB-2610
branch
from
a37e4ae to
876d146
Compare
No description provided.