From d28387cc1b9748165dcce6afe923d3bac6207aaf Mon Sep 17 00:00:00 2001 From: ItamarYuran <95186982+ItamarYuran@users.noreply.github.com> Date: Tue, 8 Oct 2024 19:12:50 +0300 Subject: [PATCH] lakectl log: add option to filter merge commits (#8142) Added the flag --no-merges to lakectl --- cmd/lakectl/cmd/common_helpers.go | 5 + cmd/lakectl/cmd/log.go | 31 +++++- docs/reference/cli.md | 1 + esti/golden/lakectl_log_no_merges.golden | 18 +++ .../lakectl_log_no_merges_amount.golden | 13 +++ esti/lakectl_test.go | 103 ++++++++++++++++++ esti/lakectl_util.go | 2 +- 7 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 esti/golden/lakectl_log_no_merges.golden create mode 100644 esti/golden/lakectl_log_no_merges_amount.golden diff --git a/cmd/lakectl/cmd/common_helpers.go b/cmd/lakectl/cmd/common_helpers.go index bf1b626ea18..c66a5e771c8 100644 --- a/cmd/lakectl/cmd/common_helpers.go +++ b/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 diff --git a/cmd/lakectl/cmd/log.go b/cmd/lakectl/cmd/log.go index a40812d66f1..349a450e714 100644 --- a/cmd/lakectl/cmd/log.go +++ b/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 ", @@ -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") diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 9513258f2f9..02f6ba7c51d 100644 --- a/docs/reference/cli.md +++ b/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 diff --git a/esti/golden/lakectl_log_no_merges.golden b/esti/golden/lakectl_log_no_merges.golden new file mode 100644 index 00000000000..63d8e3a8c99 --- /dev/null +++ b/esti/golden/lakectl_log_no_merges.golden @@ -0,0 +1,18 @@ + +ID: +Author: esti +Date: