[MINOR] Refactor hive realtime config to extend from HoodieConfig#3307
[MINOR] Refactor hive realtime config to extend from HoodieConfig#3307leoluan2009 wants to merge 5 commits intoapache:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3307 +/- ##
============================================
- Coverage 47.74% 45.65% -2.09%
- Complexity 5591 5596 +5
============================================
Files 938 999 +61
Lines 41823 43790 +1967
Branches 4213 4403 +190
============================================
+ Hits 19968 19992 +24
- Misses 20070 22015 +1945
+ Partials 1785 1783 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| <artifactId>hudi-common</artifactId> | ||
| <version>${project.version}</version> | ||
| </dependency> | ||
| <dependency> |
There was a problem hiding this comment.
hmmm. not sure if we would want hadoop-mr depend on the client-common
| .withReverseReader(false) | ||
| .withBufferSize(jobConf.getInt(HoodieRealtimeConfig.MAX_DFS_STREAM_BUFFER_SIZE_PROP, HoodieRealtimeConfig.DEFAULT_MAX_DFS_STREAM_BUFFER_SIZE)) | ||
| .withSpillableMapBasePath(jobConf.get(HoodieRealtimeConfig.SPILLABLE_MAP_BASE_PATH_PROP, HoodieRealtimeConfig.DEFAULT_SPILLABLE_MAP_BASE_PATH)) | ||
| .withBufferSize(jobConf.getInt(HoodieMemoryConfig.MAX_DFS_STREAM_BUFFER_SIZE_PROP.key(), |
There was a problem hiding this comment.
HoodieMemoryConfig has some stuff that is specific to writing, like WriteStatus . Can we leave these configs as-is in the HoodieRealtimeConfig
yihua
added
priority:medium
|
@leoluan2009 do you still have bandwidth to fix this PR? |
yihua
left a comment
yihua
left a comment
There was a problem hiding this comment.
This PR is no longer needed as the latest code on master has covered similar changes already. Closing this PR.
| string2Boolean( | ||
| config.get(HoodieRealtimeConfig.COMPACTION_LAZY_BLOCK_READ_ENABLED_PROP, | ||
| HoodieRealtimeConfig.DEFAULT_COMPACTION_LAZY_BLOCK_READ_ENABLED))) | ||
| .withReadBlocksLazily(config.getBoolean(HoodieRealtimeConfig.COMPACTION_LAZY_BLOCK_READ_ENABLED_PROP.key(), |
There was a problem hiding this comment.
.withReadBlocksLazily is already removed.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.apache.hudi</groupId> | ||
| <artifactId>hudi-client-common</artifactId> |
There was a problem hiding this comment.
HoodieMemoryConfig, HoodieCommonConfig and HoodieReaderConfig containing all "real-time" configs are already in hudi-common module on master so there is no need to add hudi-client-common module dependency.
| import org.apache.hudi.common.config.ConfigProperty; | ||
| import org.apache.hudi.common.config.HoodieConfig; | ||
|
|
||
| public class HoodieRealtimeConfig extends HoodieConfig { |
There was a problem hiding this comment.
HoodieRealtimeConfig no longer exists and these configs are moved to HoodieMemoryConfig, HoodieCommonConfig and HoodieReaderConfig.
5 participants
Tips
What is the purpose of the pull request
(For example: This pull request adds quick-start document.)
Brief change log
(for example:)
Verify this pull request
(Please pick either of the following options)
This pull request is a trivial rework / code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Committer checklist
Has a corresponding JIRA in PR title & commit
Commit message is descriptive of the change
CI is green
Necessary doc changes done or have another open PR
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.