Add --all and --repo flags to kv dump command #9874

Higangssh · 2025-12-22T17:26:12Z

Summary

This PR adds two new flags to the lakefs kv dump command as requested in #9857:

--all: Dumps all known partitions instead of just the predefined sections
--repo <name>: Dumps only the partition for a specific repository

Changes

Added --all flag to enumerate and dump all partitions from SectionMapping
Added --repo flag to dump a specific repository's partition using graveler
Added GetAllKnownPartitions() and CreateDumpWithPartitions() helper functions
Added unit tests for the new dump functionality

Usage

# Dump all partitions
lakefs kv dump --all

# Dump specific repository partition
lakefs kv dump --repo my-repo

# Existing behavior still works
lakefs kv dump --sections auth,pulls

Test

Added unit tests in pkg/kv/dump_test.go

Closes #9857

- Add --all flag to dump all known partitions - Add --repo flag to dump specific repository partition - Add unit tests for dump functionality Closes treeverse#9857

CLAassistant · 2025-12-22T17:26:18Z

All committers have signed the CLA.

arielshaqed

Thanks! Let's test it, and we'll go over now tests later.

arielshaqed · 2025-12-29T18:07:09Z

cmd/lakefs/cmd/kv.go

  kv        - kv internal metadata

-Default: all supported sections (auth, pulls, kv)`,
+Default: all supported sections (auth, pulls, kv)


Let's now avoid the word "all" here, given that it's but the same as the flag.

arielshaqed · 2025-12-29T18:10:20Z

cmd/lakefs/cmd/kv.go

+		var dump *kv.DumpFormat
+
+		// Handle different dump modes
+		if allFlag {


This would be easier for me to read as a switch statement.

arielshaqed · 2025-12-29T18:11:45Z

cmd/lakefs/cmd/kv.go

+	var repoData graveler.RepositoryData
+	_, err := kv.GetMsg(ctx, kvStore, graveler.RepositoriesPartition(), []byte(repoPath), &repoData)
+	if err != nil {
+		if errors.Is(err, kv.ErrNotFound) {


Use just errors.Is , which is false for nil. This style is used throughout lakeFS.

arielshaqed · 2025-12-29T18:13:11Z

pkg/kv/dump_test.go

+
+func TestCreateDumpWithPartitions(t *testing.T) {
+	ctx := t.Context()
+	store := kvtest.GetStore(ctx, t)


Surprised this works tbh, cool!

arielshaqed · 2025-12-29T18:13:41Z

pkg/kv/dump_test.go

+		dump, err := kv.CreateDumpWithPartitions(ctx, store, []string{"partition1", "partition2"})
+		require.NoError(t, err)
+		require.NotNil(t, dump)
+		require.Len(t, dump.Entries, 3)


Can you verify the returned values?

- Fix duplicate graveler import - Use static error variables for linter compliance - Refactor if/else to switch statement - Simplify error handling with errors.Is pattern - Add value verification in dump tests

Higangssh · 2025-12-30T13:58:29Z

Hi @arielshaqed, thanks for taking the time to review!

I've pushed a new commit addressing your feedback:

Changed "all supported sections" to "supported sections" to avoid confusion with the --all flag
Refactored the conditional logic to a switch statement
Simplified error handling with errors.Is
Added static error variables (fixes linter errors too)
Added value verification in the tests

Let me know if there's anything else I should change!

arielshaqed

Thanks! I'll get CI to pass (but please go over my comments) and then pull.

arielshaqed · 2025-12-30T15:12:02Z

pkg/kv/dump.go

 	"kv":    {"kv-internal-metadata"},         // kv internal metadata
 }

+// GetAllKnownPartitions returns all partitions defined in SectionMapping


Why do we need this? CreateDump still tests len(partitions) == 0 and collects all known partitions.

arielshaqed · 2025-12-30T15:28:23Z

pkg/kv/dump_test.go

+	require.Equal(t, len(expectedPartitions), len(partitions), "should return all unique partitions")
+
+	for _, partition := range partitions {
+		require.True(t, expectedPartitions[partition], "partition %s should be in expected list", partition)
+	}
+}


Use testify.ElementsMatch.

- Use CreateDump with empty sections for --all flag - Remove redundant GetAllKnownPartitions function and test - Fix gofmt formatting

Higangssh · 2025-12-30T16:24:51Z

Hi @arielshaqed, I've addressed your comments:

Removed GetAllKnownPartitions and now using CreateDump with empty sections instead
Removed the redundant test as well

Thanks again for the helpful feedback!

arielshaqed · 2025-12-30T16:34:59Z

One last question - have you run it to see that it works (beyond unit tests, which are all well and good but...)?

Higangssh · 2025-12-30T17:33:37Z

Yes! I've tested it locally using quickstart mode:

lakefs kv dump --quickstart → outputs JSON correctly
lakefs kv dump --all --quickstart --pretty → works with pretty formatting
lakefs kv dump --repo non-existent --quickstart → returns proper error message

All flags work as expected. The dump is empty since quickstart has no data,
but the command execution and output format are correct

Add --all and --repo flags to kv dump command

72a98ec

- Add --all flag to dump all known partitions - Add --repo flag to dump specific repository partition - Add unit tests for dump functionality Closes treeverse#9857

github-actions bot added area/testing Improvements or additions to tests area/KV Improvements to the KV store implementation labels Dec 22, 2025

arielshaqed requested changes Dec 29, 2025

View reviewed changes

Address PR review feedback

33d7ff3

- Fix duplicate graveler import - Use static error variables for linter compliance - Refactor if/else to switch statement - Simplify error handling with errors.Is pattern - Add value verification in dump tests

Higangssh requested a review from arielshaqed December 30, 2025 13:58

arielshaqed approved these changes Dec 30, 2025

View reviewed changes

Remove GetAllKnownPartitions, use CreateDump instead

458a9cd

- Use CreateDump with empty sections for --all flag - Remove redundant GetAllKnownPartitions function and test - Fix gofmt formatting

arielshaqed added area/UI Improvements or additions to UI include-changelog PR description should be included in next release changelog labels Dec 30, 2025

Add --all and --repo flags to kv dump command #9874

Are you sure you want to change the base?

Add --all and --repo flags to kv dump command #9874

Conversation

Higangssh commented Dec 22, 2025

Summary

Changes

Usage

Uh oh!

CLAassistant commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arielshaqed left a comment

Choose a reason for hiding this comment

Uh oh!

arielshaqed Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

arielshaqed Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

arielshaqed Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

arielshaqed Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

arielshaqed Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Higangssh commented Dec 30, 2025

Uh oh!

arielshaqed left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arielshaqed Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

arielshaqed Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Higangssh commented Dec 30, 2025

Uh oh!

arielshaqed commented Dec 30, 2025

Uh oh!

Higangssh commented Dec 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CLAassistant commented Dec 22, 2025 •

edited

Loading

arielshaqed left a comment •

edited

Loading