voonhous commented on code in PR #18816:
URL: https://github.com/apache/hudi/pull/18816#discussion_r3887539682
##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestArchivedCommitsCommand.java:
##########
@@ -168,6 +159,7 @@ public void testShowArchivedCommits() {
/**
* Test for command: show archived commits.
*/
+ @Disabled("TODO: HUDI-7614 - ArchivedCommitsCommand reads old
HoodieLogFormat but v9 tables use LSMTimelineWriter")
Review Comment:
Done -- `ArchivedCommitsCommand` now reads through
`metaClient.getArchivedTimeline()`, which dispatches to `ArchivedTimelineV2`
for the LSM timeline and `ArchivedTimelineV1` for older tables, then uses the
typed read API per action (`readCommitMetadataToAvro`, `readCleanMetadata`,
`readRollbackMetadata`, `readSavepointMetadata`, `readCompactionPlan`,
`readReplaceCommitMetadataToAvro`). That is the same pattern
`CommitsCommand.showArchivedCommits` and
`CompactionCommand.compactionShowArchived` already use. Both `@Disabled` tests
are re-enabled and passing.
Two things worth your eye:
1. **Output changes.** The legacy format archived requested, inflight and
completed as separate entries, so `show archived commits` printed 3 rows per
commit; the LSM timeline holds one entry per instant, so it now prints 1. The
stats command goes from 3 rows per partition to 1. The tests were rewritten for
the new counts with the same constants and the same full-table comparison, not
weakened.
2. **`--archiveFolderPattern`** has no timeline-API equivalent, so it now
explicitly selects the legacy log-format reader for pre-v8 archive folders,
with the help text saying so. Removing it would have broken CLI compatibility
and making it a no-op would silently ignore user input -- happy to change if
you would rather it go.
Also folded in: partition-keyed maps are sorted before rendering. The
metadata column was previously leaking JVM hash iteration order, which happened
to look sorted with the old Utf8 keys and does not with String keys.
This covers `ArchivedCommitsCommand` only. `TestRepairsCommand` (2),
`TestRestoresCommand` (2) and `TestArchiveCommand` (1) are still disabled under
HUDI-7614 -- different commands, different root causes. Say the word if you
want those in this patch too.
--
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]