lakectl log: add option to filter merge commits (#8142)

Added the flag --no-merges to lakectl

cmd/lakectl/cmd/common_helpers.go

@@ -41,6 +41,11 @@ const (
 	internalPageSize           = 1000 // when retrieving all records, use this page size under the hood
 	defaultAmountArgumentValue = 100  // when no amount is specified, use this value for the argument
 
+	// when using --no-merges & amount, this const is the upper limit to use heuristic.
+	// The heuristic asks for 3*amount results since some will filter out
+	maxAmountNoMerges = 333
+	noMergesHeuristic = 3
+
 	defaultPollInterval = 3 * time.Second // default interval while pulling tasks status
 	minimumPollInterval = time.Second     // minimum interval while pulling tasks status
 	defaultPollTimeout  = time.Hour       // default expiry for pull status with no update

cmd/lakectl/cmd/log.go

@@ -66,6 +66,19 @@ func (d *dotWriter) Write(commits []apigen.Commit) {
 	}
 }
 
+// filter merge commits, used for --no-merges flag
+func filterMergeCommits(commits []apigen.Commit) []apigen.Commit {
+	filteredCommits := make([]apigen.Commit, 0, len(commits))
+
+	// iterating through data.Commit, appending every instance with 1 or less parents.
+	for _, commit := range commits {
+		if len(commit.Parents) <= 1 {
+			filteredCommits = append(filteredCommits, commit)
+		}
+	}
+	return filteredCommits
+}
+
 // logCmd represents the log command
 var logCmd = &cobra.Command{
 	Use:               "log <branch URI>",
@@ -80,6 +93,7 @@ var logCmd = &cobra.Command{
 		limit := Must(cmd.Flags().GetBool("limit"))
 		since := Must(cmd.Flags().GetString("since"))
 		dot := Must(cmd.Flags().GetBool("dot"))
+		noMerges := Must(cmd.Flags().GetBool("no-merges"))
 		firstParent := Must(cmd.Flags().GetBool("first-parent"))
 		objects := Must(cmd.Flags().GetStringSlice("objects"))
 		prefixes := Must(cmd.Flags().GetStringSlice("prefixes"))
@@ -100,6 +114,10 @@ var logCmd = &cobra.Command{
 		if amountForPagination <= 0 {
 			amountForPagination = internalPageSize
 		}
+		// case --no-merges & --amount, ask for more results since some will filter out
+		if noMerges && amountForPagination < maxAmountNoMerges {
+			amountForPagination *= noMergesHeuristic
+		}
 		logCommitsParams := &apigen.LogCommitsParams{
 			After:       apiutil.Ptr(apigen.PaginationAfter(after)),
 			Amount:      apiutil.Ptr(apigen.PaginationAmount(amountForPagination)),
@@ -151,13 +169,23 @@ var logCmd = &cobra.Command{
 				},
 			}
 
+			// case --no-merges, filter commits and subtract that amount from amount desired
+			if noMerges {
+				data.Commits = filterMergeCommits(data.Commits)
+				// case user asked for a specified amount
+				if amount > 0 {
+					data.Commits = data.Commits[:amount]
+				}
+			}
+			amount -= len(data.Commits)
+
 			if dot {
 				graph.Write(data.Commits)
 			} else {
 				Write(commitsTemplate, data)
 			}
 
-			if amount != 0 {
+			if amount <= 0 {
 				// user request only one page
 				break
 			}
@@ -177,6 +205,7 @@ func init() {
 	logCmd.Flags().String("after", "", "show results after this value (used for pagination)")
 	logCmd.Flags().Bool("dot", false, "return results in a dotgraph format")
 	logCmd.Flags().Bool("first-parent", false, "follow only the first parent commit upon seeing a merge commit")
+	logCmd.Flags().Bool("no-merges", false, "skip merge commits")
 	logCmd.Flags().Bool("show-meta-range-id", false, "also show meta range ID")
 	logCmd.Flags().StringSlice("objects", nil, "show results that contains changes to at least one path in that list of objects. Use comma separator to pass all objects together")
 	logCmd.Flags().StringSlice("prefixes", nil, "show results that contains changes to at least one path in that list of prefixes. Use comma separator to pass all prefixes together")

docs/reference/cli.md

@@ -2322,6 +2322,7 @@ lakectl log --dot lakefs://example-repository/main | dot -Tsvg > graph.svg
       --first-parent         follow only the first parent commit upon seeing a merge commit
   -h, --help                 help for log
       --limit                limit result just to amount. By default, returns whether more items are available.
+      --no-merges            skip merge commits
       --objects strings      show results that contains changes to at least one path in that list of objects. Use comma separator to pass all objects together
       --prefixes strings     show results that contains changes to at least one path in that list of prefixes. Use comma separator to pass all prefixes together
       --show-meta-range-id   also show meta range ID

esti/golden/lakectl_log_no_merges.golden

@@ -0,0 +1,18 @@
+
+ID:            <COMMIT_ID>
+Author:        esti
+Date:          <DATE> <TIME> <TZ>
+
+	${SECOND_MESSAGE}
+
+ID:            <COMMIT_ID>
+Author:        esti
+Date:          <DATE> <TIME> <TZ>
+
+	${MESSAGE}
+
+ID:            <COMMIT_ID>
+Date:          <DATE> <TIME> <TZ>
+
+	Repository created
+

esti/golden/lakectl_log_no_merges_amount.golden

@@ -0,0 +1,13 @@
+
+ID:            <COMMIT_ID>
+Author:        esti
+Date:          <DATE> <TIME> <TZ>
+
+	${SECOND_MESSAGE}
+
+ID:            <COMMIT_ID>
+Author:        esti
+Date:          <DATE> <TIME> <TZ>
+
+	${MESSAGE}
+

esti/lakectl_test.go

@@ -338,6 +338,109 @@ func TestLakectlMergeAndStrategies(t *testing.T) {
 	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" fs ls lakefs://"+repoName+"/"+featureBranch+"/", false, "lakectl_fs_ls_1_file", vars)
 }
 
+func TestLakectlLogNoMergesWithCommitsAndMerges(t *testing.T) {
+	repoName := generateUniqueRepositoryName()
+	storage := generateUniqueStorageNamespace(repoName)
+	vars := map[string]string{
+		"REPO":    repoName,
+		"STORAGE": storage,
+		"BRANCH":  mainBranch,
+	}
+
+	featureBranch := "feature"
+	branchVars := map[string]string{
+		"REPO":          repoName,
+		"STORAGE":       storage,
+		"SOURCE_BRANCH": mainBranch,
+		"DEST_BRANCH":   featureBranch,
+		"BRANCH":        featureBranch,
+	}
+
+	filePath1 := "file1"
+	filePath2 := "file2"
+
+	// create repo with 'main' branch
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" repo create lakefs://"+repoName+" "+storage, false, "lakectl_repo_create", vars)
+
+	// upload 'file1' and commit
+	vars["FILE_PATH"] = filePath1
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" fs upload -s files/ro_1k lakefs://"+repoName+"/"+mainBranch+"/"+filePath1, false, "lakectl_fs_upload", vars)
+	commitMessage := "first commit to main"
+	vars["MESSAGE"] = commitMessage
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" commit lakefs://"+repoName+"/"+mainBranch+" -m \""+commitMessage+"\"", false, "lakectl_commit", vars)
+
+	// create new branch 'feature'
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" branch create lakefs://"+repoName+"/"+featureBranch+" --source lakefs://"+repoName+"/"+mainBranch, false, "lakectl_branch_create", branchVars)
+
+	// upload 'file2' to feature branch and commit
+	branchVars["FILE_PATH"] = filePath2
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" fs upload -s files/ro_1k lakefs://"+repoName+"/"+featureBranch+"/"+filePath2, false, "lakectl_fs_upload", branchVars)
+	commitMessage = "second commit to feature branch"
+	branchVars["MESSAGE"] = commitMessage
+	vars["SECOND_MESSAGE"] = commitMessage
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" commit lakefs://"+repoName+"/"+featureBranch+" -m \""+commitMessage+"\"", false, "lakectl_commit", branchVars)
+
+	// merge feature into main
+	branchVars["SOURCE_BRANCH"] = featureBranch
+	branchVars["DEST_BRANCH"] = mainBranch
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" merge lakefs://"+repoName+"/"+featureBranch+" lakefs://"+repoName+"/"+mainBranch, false, "lakectl_merge_success", branchVars)
+
+	// log the commits without merges
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" log lakefs://"+repoName+"/"+mainBranch+" --no-merges", false, "lakectl_log_no_merges", vars)
+
+}
+
+func TestLakectlLogNoMergesAndAmount(t *testing.T) {
+	repoName := generateUniqueRepositoryName()
+	storage := generateUniqueStorageNamespace(repoName)
+	vars := map[string]string{
+		"REPO":    repoName,
+		"STORAGE": storage,
+		"BRANCH":  mainBranch,
+	}
+
+	featureBranch := "feature"
+	branchVars := map[string]string{
+		"REPO":          repoName,
+		"STORAGE":       storage,
+		"SOURCE_BRANCH": mainBranch,
+		"DEST_BRANCH":   featureBranch,
+		"BRANCH":        featureBranch,
+	}
+
+	filePath1 := "file1"
+	filePath2 := "file2"
+
+	// create repo with 'main' branch
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" repo create lakefs://"+repoName+" "+storage, false, "lakectl_repo_create", vars)
+
+	// upload 'file1' and commit
+	vars["FILE_PATH"] = filePath1
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" fs upload -s files/ro_1k lakefs://"+repoName+"/"+mainBranch+"/"+filePath1, false, "lakectl_fs_upload", vars)
+	commitMessage := "first commit to main"
+	vars["MESSAGE"] = commitMessage
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" commit lakefs://"+repoName+"/"+mainBranch+" -m \""+commitMessage+"\"", false, "lakectl_commit", vars)
+
+	// create new branch 'feature'
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" branch create lakefs://"+repoName+"/"+featureBranch+" --source lakefs://"+repoName+"/"+mainBranch, false, "lakectl_branch_create", branchVars)
+
+	// upload 'file2' to feature branch and commit
+	branchVars["FILE_PATH"] = filePath2
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" fs upload -s files/ro_1k lakefs://"+repoName+"/"+featureBranch+"/"+filePath2, false, "lakectl_fs_upload", branchVars)
+	commitMessage = "second commit to feature branch"
+	branchVars["MESSAGE"] = commitMessage
+	vars["SECOND_MESSAGE"] = commitMessage
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" commit lakefs://"+repoName+"/"+featureBranch+" -m \""+commitMessage+"\"", false, "lakectl_commit", branchVars)
+
+	// merge feature into main
+	branchVars["SOURCE_BRANCH"] = featureBranch
+	branchVars["DEST_BRANCH"] = mainBranch
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" merge lakefs://"+repoName+"/"+featureBranch+" lakefs://"+repoName+"/"+mainBranch, false, "lakectl_merge_success", branchVars)
+
+	// log the commits without merges
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" log lakefs://"+repoName+"/"+mainBranch+" --no-merges --amount=2", false, "lakectl_log_no_merges_amount", vars)
+
+}
 func TestLakectlAnnotate(t *testing.T) {
 	repoName := generateUniqueRepositoryName()
 	storage := generateUniqueStorageNamespace(repoName)

esti/lakectl_util.go

@@ -232,7 +232,7 @@ func runCmdAndVerifyResult(t *testing.T, cmd string, expectFail bool, isTerminal
 	t.Helper()
 	expanded, err := expandVariables(expected, vars)
 	if err != nil {
-		t.Fatal("Failed to extract variables for:", cmd)
+		t.Fatalf("Failed to extract variables for: \"%s\": %s", cmd, err)
 	}
 	sanitizedResult := runCmd(t, cmd, expectFail, isTerminal, vars)