[Refactor] Introduce Iceberg TableRuntimePlugin#4073
[Refactor] Introduce Iceberg TableRuntimePlugin#4073majin1102 wants to merge 2 commits intoapache:masterfrom
Conversation
majin1102
force-pushed
the
tableruntimeplugin
branch
3 times, most recently
from
0f1a1a0 to
40120af
Compare
majin1102
force-pushed
the
tableruntimeplugin
branch
from
c3ccf93 to
72eef46
Compare
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #4073 +/- ##
============================================
- Coverage 28.63% 22.39% -6.24%
+ Complexity 3942 2552 -1390
============================================
Files 654 458 -196
Lines 52293 42116 -10177
Branches 6621 5917 -704
============================================
- Hits 14973 9433 -5540
+ Misses 36230 31871 -4359
+ Partials 1090 812 -278
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| tableService.addHandlerChain(chain); | ||
| } | ||
| private List<TableRuntimePlugin> initTablePlugins() { | ||
| LOG.info("Setting up AMS table executors..."); |
There was a problem hiding this comment.
Setting up AMS Table Plugins ...
| private List<TableRuntimePlugin> initTablePlugins() { | ||
| LOG.info("Setting up AMS table executors..."); | ||
| InlineTableExecutors.getInstance().setup(tableService, serviceConfig); | ||
| IcebergTablePlugin icebergTablePlugin = |
There was a problem hiding this comment.
How to load different table runtime plugins?
|
|
||
| @Override | ||
| public boolean accept(ServerTableIdentifier tableIdentifier) { | ||
| return tableIdentifier.getFormat() == TableFormat.ICEBERG |
There was a problem hiding this comment.
IcebergTablePlugin will accept other formats? It should not be named as IcebergTablePlugin
There was a problem hiding this comment.
I think TableFormatPlugin should not be implemented in the current PR. The current PR is for renaming and organizing, which I can provide later
|
|
||
| @Override | ||
| public void initialize(List<TableRuntime> tableRuntimes) { | ||
| if (headHandler != null) { |
There was a problem hiding this comment.
Already checked null in construct function
| } | ||
| private void initTableRuntimePlugins() { | ||
| List<TableRuntime> tableRuntimes = new ArrayList<>(tableRuntimeMap.values()); | ||
| tableRuntimePlugins.forEach(plugin -> plugin.initialize(tableRuntimes)); |
There was a problem hiding this comment.
Plugin is invoked in initTableRuntimes when create tableRuntime. Should we check the method workflow?
4 participants
Why are the changes needed?
Refactoring work for format processing extensions
Brief change log
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before making a pull request
Documentation