rdblue commented on code in PR #15561:
URL: https://github.com/apache/iceberg/pull/15561#discussion_r2921132154
##########
core/src/main/java/org/apache/iceberg/rest/RESTTableScan.java:
##########
@@ -131,14 +131,16 @@ protected TableScan newRefinedScan(
tableIdentifier,
resourcePaths,
supportedEndpoints,
- io(),
+ null != scanFileIO ? scanFileIO : tableIO,
catalogProperties,
hadoopConf);
}
@Override
- protected FileIO io() {
- return null != fileIOForPlanId ? fileIOForPlanId : tableIO;
+ public FileIO io() {
+ Preconditions.checkState(
+ null != scanFileIO, "Cannot use FileIO because planFiles() must be
called first");
Review Comment:
How about `"Cannot access FileIO: planFiles() must be called first"`? Or
maybe `"FileIO is not available: planFiles() must be called first"`? I think I
prefer the second. And both fit with the style of error messages for the
project, which is to be direct ("FileIO is not available") and then describe
why ("planFiles() must be called first") with a clear distinction.
I think it is good to avoid full sentences because that makes error messages
longer and more complicated. Making error messages sentences introduces
problems like needing a subject or to conjugate something in an awkward way. In
this case, it's just shorter to omit "because" and use the standard format of
`"What: why"`.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
