HeartSaVioR commented on a change in pull request #1429:
URL: https://github.com/apache/iceberg/pull/1429#discussion_r484675569
##########
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:
The reason I lower down the log level only for
NoSuchIcebergTableException is to avoid noisy and misleading log message for
this case; 1) use default catalog in Spark 3, with hive catalog 2) try to load
the table which is "not" the iceberg table.
... and probably we just want to lower down the log level (or not log at
all) with NoSuchTableException, as the condition 2) has two different
situations 2.a) table exists in HMS but isn't an iceberg table 2.b) table
doesn't exist. 2.b is probably also the valid case if Spark doesn't connect
with Hive via remote HMS, which we also don't want to let end users see this
noise.
WDYT? (for all reviewers)
----------------------------------------------------------------
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]
