Common: Validate DynConstructors implementation types#15912
Conversation
manuzhang
force-pushed
the
common-dynconstructors-assignability-check
branch
from
9fc73a9 to
00e78f0
Compare
manuzhang
force-pushed
the
common-dynconstructors-assignability-check
branch
from
00e78f0 to
10d24e6
Compare
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
| String.format("Cannot initialize FileIO, %s does not implement FileIO.", impl), e); | ||
| } | ||
|
|
||
| FileIO fileIO = ctor.newInstance(); |
There was a problem hiding this comment.
DynConstructors now throws IllegalArgumentException in impl()/hiddenImpl() before newInstance() runs, so this ClassCastException branch is unreachable - and so are the identical catches left in the sibling loaders: loadCatalog (line 288) and loadMetricsReporter (line 538) in this file, plus LocationProviders.java:63, rest/HTTPClient.java:461, and azure AdlsTokenCredentialProviders.java:74, all paths whose tests this PR already updated to the new message. The same now-dead pattern also exists in other builder(base) callers (AwsClientFactories, S3FileIOAwsClientFactories, AuthManagers, LockManagers, EncryptionUtil, Dell/Aliyun factories). Consider removing them here for consistency.
2 participants
Follow-up of #10542