fix(core): gate the timeline path configs on the table version - #19566
Draft
yihua wants to merge 1 commit into
Draft
fix(core): gate the timeline path configs on the table version#19566yihua wants to merge 1 commit into
yihua wants to merge 1 commit into
Conversation
hoodie.timeline.path and hoodie.timeline.history.path describe the timeline layout version 2 folders, which a table below version 8 does not have. Neither declared a since version, and dropInvalidConfigs only strips configs that declare one, so both were persisted into the hoodie.properties of tables whose timeline lives directly under .hoodie. Declare their introducing version so they are dropped below table version 8.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19566 +/- ##
=========================================
Coverage 76.84% 76.84%
+ Complexity 32379 32378 -1
=========================================
Files 2522 2522
Lines 139106 139108 +2
Branches 16713 16713
=========================================
+ Hits 106892 106895 +3
+ Misses 24621 24617 -4
- Partials 7593 7596 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the issue this Pull Request addresses
Closes #19562
Summary and Changelog
hoodie.timeline.pathandhoodie.timeline.history.pathdescribe the timeline layout version 2 folders, introduced with table version 8. A table below that version keeps its timeline directly under.hoodieand resolves its archived timeline throughhoodie.archivelog.folder, so neither config has meaning there.Neither declared a
sinceVersion, andHoodieTableConfig.dropInvalidConfigs()only strips a config that declares one, so both were persisted into thehoodie.propertiesof tables that do not use the layout they describe.RECORD_MERGE_MODEdeclares.sinceVersion("1.0.0")and is correctly dropped from a version 6 table, which is the behaviour these two were missing.Changes:
sinceVersion("1.0.0")onTIMELINE_PATHandTIMELINE_HISTORY_PATHso the existing gate drops them below table version 8.TestHoodieTableConfigcoverage across table versions 6, 7, 8 and 9, with literal per-version expectations, also asserting thatARCHIVELOG_FOLDERis retained at every version.hoodie.table.formatwas considered alongside these and deliberately left alone. Its introducing version is not the version at which it becomes meaningful: the table format SPI is orthogonal to the table version, nothing in the codebase gates a custom format on table version 9, and no upgrade handler restores the config. Gating it onsinceVersionwould silently and permanently discard a custom format on any table below version 9, so excluding that config needs a different mechanism.Impact
Tables created below version 8 no longer carry the two timeline layout version 2 configs in their properties file.
No read path changes. Every consumer resolves these through
getStringOrDefault, and the only provider that reads them,TimelinePathProviderV2, is selected exclusively at layout version 2 where the configs are retained.TimelinePathProviderV1reads neither. Resolved timeline and archive paths were verified unchanged at table versions 6, 7, 8 and 9.Existing tables are not modified; the gate applies when properties are written. Upgrade is unaffected, since
SevenToEightUpgradeHandlerre-addsTIMELINE_PATHthroughHoodieTableConfig.update(), which does not run the gate.One cosmetic inconsistency remains: a table upgraded from 6 or 7 to 8 will not carry
hoodie.timeline.history.path, because that handler addshoodie.timeline.pathbut not its sibling. Both resolve to the same default either way.No public API change.
Risk Level
low
Confined to which keys are written into
hoodie.properties. Verified that no consumer reads either config through a null-returning accessor, and that resolved paths are identical before and after across four table versions. Reverting the change fails the added test at versions 6 and 7.Documentation Update
none
No config is added or removed and no default value changes; this records the version at which two existing configs were introduced.
Contributor's checklist