HeartSaVioR commented on a change in pull request #1429:
URL: https://github.com/apache/iceberg/pull/1429#discussion_r484671453
##########
File path:
core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java
##########
@@ -75,7 +76,14 @@ public TableMetadata refresh() {
try {
doRefresh();
} catch (NoSuchTableException e) {
- LOG.warn("Could not find the table during refresh, setting current
metadata to null", e);
+ // Adjust log level according to the type of exception, as it might be
intentional to call
+ // the method without determining the type of table in prior (like
SparkSessionCatalog),
+ // and in such case it may not be an error case if the table is not an
Iceberg table.
+ if (e instanceof NoSuchIcebergTableException) {
Review comment:
Oh probably there's some miscommunication here.
I'm a contributor (same as you) and don't know about the
rationalization/intention of the codebase around here. My changes so far have
been intended to fix something I encountered during experiments, not from
learning on the codebase and making something beauty. (I'd love to if I'm
qualified, but I lack of knowledge on the codebase so far.)
Also I'm trying to change smaller while contributing so that I don't break
any intentions behind the wall. I'm self-commenting in the PR to ask about the
viewpoint from committers, to confirm before making change.
Back to the question, it might be OK to just remove the log message if we
have faith on the callers to handle the exception well. We are throwing the
exception in the method which says callers also have a chance to log it
properly, swallow if it doesn't worth to log. That is also a question for
reviewers as well.
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
