Skip to content

Commit

Permalink
cmd: remove any FindProject() redundant call
Browse files Browse the repository at this point in the history
It turns out that not only the internal gitlab code was calling
FindProject() without need (fixed on [1]). The commands themselves were
calling the FindProject() before requesting an action from an internal
gitlab module code. With that, there were places (MRCreate, IssueCreate,
...) that the FindProject() function was being called twice before actually
handing the request off to go-gitlab module.

This commit get rid of all FindProject() calls that were (1) redundant or
(2) not really needed, since GitLab's API accept both the project name or
the project number ID as input. Hopefully, this change will make many
different command calls much faster (with one or even two less API calls).

[1] #761

Signed-off-by: Bruno Meneguele <bmeneg@redhat.com>
  • Loading branch information
bmeneg committed Oct 28, 2021
1 parent da0c834 commit 44ebecf
Show file tree
Hide file tree
Showing 20 changed files with 41 additions and 141 deletions.
8 changes: 1 addition & 7 deletions cmd/ci_artifacts.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,7 @@ var ciArtifactsCmd = &cobra.Command{
log.Fatal(err)
}

project, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}
projectID := project.ID

r, outpath, err := lab.CIArtifacts(projectID, pipelineID, jobName, path, followBridge, bridgeName)
r, outpath, err := lab.CIArtifacts(rn, pipelineID, jobName, path, followBridge, bridgeName)
if err != nil {
log.Fatal(err)
}
Expand Down
29 changes: 11 additions & 18 deletions cmd/ci_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,7 @@ var ciCreateCmd = &cobra.Command{
if err != nil {
log.Fatal(err)
}
project, err := lab.GetProject(pid)
if err != nil {
log.Fatal(err)
}
fmt.Printf("%s/pipelines/%d\n", project.WebURL, pipeline.ID)
fmt.Println(pipeline.WebURL)
},
}

Expand Down Expand Up @@ -89,41 +85,38 @@ var ciTriggerCmd = &cobra.Command{
if err != nil {
log.Fatal(err)
}
project, err := lab.GetProject(pid)
if err != nil {
log.Fatal(err)
}
fmt.Printf("%s/pipelines/%d\n", project.WebURL, pipeline.ID)
fmt.Println(pipeline.WebURL)
},
}

func getCIRunOptions(cmd *cobra.Command, args []string) (interface{}, string, error) {
func getCIRunOptions(cmd *cobra.Command, args []string) (string, string, error) {
branch, err := git.CurrentBranch()
if err != nil {
return nil, "", err
return "", "", err
}
var pid interface{}
if len(args) > 0 {
branch = args[0]
}

var pid string

remote := determineSourceRemote(branch)
rn, err := git.PathWithNamespace(remote)
if err != nil {
return nil, "", err
return "", "", err
}
pid = rn

project, err := cmd.Flags().GetString("project")
if err != nil {
return nil, "", err
return "", "", err
}
if project != "" {
p, err := lab.FindProject(project)
_, err := lab.FindProject(project)
if err != nil {
return nil, "", err
return "", "", err
}
pid = p.ID
pid = project
}
return pid, branch, nil
}
Expand Down
8 changes: 4 additions & 4 deletions cmd/ci_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func Test_ciRun(t *testing.T) {
t.Log(string(b))
t.Fatal(err)
}
require.Regexp(t, `^https://gitlab.com/lab-testing/test/pipelines/\d+`, string(b))
require.Regexp(t, `^https://gitlab.com/lab-testing/test/-/pipelines/\d+`, string(b))
}

func Test_parseCIVariables(t *testing.T) {
Expand Down Expand Up @@ -77,7 +77,7 @@ func Test_getCIRunOptions(t *testing.T) {
desc string
cmdFunc func()
args []string
expectedProject interface{}
expectedProject string
expectedBranch string
expectedErr string
}{
Expand Down Expand Up @@ -111,7 +111,7 @@ func Test_getCIRunOptions(t *testing.T) {
ciTriggerCmd.Flags().Set("project", "zaquestion/test")
},
[]string{},
4181224, // https://gitlab.com/zaquestion/test project ID
"zaquestion/test", // https://gitlab.com/zaquestion/test project name
"master",
"",
},
Expand All @@ -121,7 +121,7 @@ func Test_getCIRunOptions(t *testing.T) {
ciTriggerCmd.Flags().Set("project", "barfasdfasdf")
},
[]string{},
nil, // https://gitlab.com/zaquestion/test project ID
"", // https://gitlab.com/zaquestion/test project name
"",
"GitLab project not found, verify you have access to the requested resource",
},
Expand Down
12 changes: 3 additions & 9 deletions cmd/ci_trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,23 +73,17 @@ var ciTraceCmd = &cobra.Command{
log.Fatal(err)
}

project, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}
projectID = project.ID

pager := newPager(cmd.Flags())
defer pager.Close()

err = doTrace(context.Background(), os.Stdout, projectID, pipelineID, jobName)
err = doTrace(context.Background(), os.Stdout, rn, pipelineID, jobName)
if err != nil {
log.Fatal(err)
}
},
}

func doTrace(ctx context.Context, w io.Writer, pid interface{}, pipelineID int, name string) error {
func doTrace(ctx context.Context, w io.Writer, projID string, pipelineID int, name string) error {
var (
once sync.Once
offset int64
Expand All @@ -98,7 +92,7 @@ func doTrace(ctx context.Context, w io.Writer, pid interface{}, pipelineID int,
if ctx.Err() == context.Canceled {
break
}
trace, job, err := lab.CITrace(pid, pipelineID, name, followBridge, bridgeName)
trace, job, err := lab.CITrace(projID, pipelineID, name, followBridge, bridgeName)
if err != nil || job == nil || trace == nil {
return errors.Wrap(err, "failed to find job")
}
Expand Down
8 changes: 2 additions & 6 deletions cmd/ci_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
)

var (
projectID int
projectID string
pipelineID int
)

Expand Down Expand Up @@ -78,11 +78,7 @@ var ciViewCmd = &cobra.Command{
log.Fatal(err)
}

project, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}
projectID = project.ID
projectID = rn
root := tview.NewPages()
root.SetBorderPadding(1, 1, 2, 2)

Expand Down
9 changes: 2 additions & 7 deletions cmd/issue_close.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,18 @@ var issueCloseCmd = &cobra.Command{
log.Fatal(err)
}

p, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}

dupID, _ := cmd.Flags().GetString("duplicate")
if dupID != "" {
if !strings.Contains(dupID, "#") {
dupID = "#" + dupID
}
err = lab.IssueDuplicate(p.ID, int(id), dupID)
err = lab.IssueDuplicate(rn, int(id), dupID)
if err != nil {
log.Fatal(err)
}
fmt.Printf("Issue #%d closed as duplicate of %s\n", id, dupID)
} else {
err = lab.IssueClose(p.ID, int(id))
err = lab.IssueClose(rn, int(id))
if err != nil {
log.Fatal(err)
}
Expand Down
7 changes: 1 addition & 6 deletions cmd/issue_reopen.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,7 @@ var issueReopenCmd = &cobra.Command{
log.Fatal(err)
}

p, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}

err = lab.IssueReopen(p.ID, int(id))
err = lab.IssueReopen(rn, int(id))
if err != nil {
log.Fatal(err)
}
Expand Down
7 changes: 1 addition & 6 deletions cmd/issue_subscribe.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,7 @@ var issueSubscribeCmd = &cobra.Command{
log.Fatal(err)
}

p, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}

err = lab.IssueSubscribe(p.ID, int(id))
err = lab.IssueSubscribe(rn, int(id))
if err != nil {
log.Fatal(err)
}
Expand Down
7 changes: 1 addition & 6 deletions cmd/issue_unsubscribe.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,7 @@ var issueUnsubscribeCmd = &cobra.Command{
log.Fatal(err)
}

p, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}

err = lab.IssueUnsubscribe(p.ID, int(id))
err = lab.IssueUnsubscribe(rn, int(id))
if err != nil {
log.Fatal(err)
}
Expand Down
7 changes: 1 addition & 6 deletions cmd/mr_close.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,7 @@ var mrCloseCmd = &cobra.Command{
log.Fatal(err)
}

p, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}

err = lab.MRClose(p.ID, int(id))
err = lab.MRClose(rn, int(id))
if err != nil {
log.Fatal(err)
}
Expand Down
13 changes: 4 additions & 9 deletions cmd/mr_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ func init() {
)
}

func verifyRemoteBranch(projectID int, branch string) error {
if _, err := lab.GetCommit(projectID, branch); err != nil {
func verifyRemoteBranch(projID string, branch string) error {
if _, err := lab.GetCommit(projID, branch); err != nil {
return fmt.Errorf("%s is not a valid reference", branch)
}
return nil
Expand Down Expand Up @@ -174,13 +174,8 @@ func runMRCreate(cmd *cobra.Command, args []string) {
log.Fatal(err)
}

sourceProject, err := lab.FindProject(sourceProjectName)
if err != nil {
log.Fatal(err)
}

// verify the source branch in remote project
err = verifyRemoteBranch(sourceProject.ID, sourceBranch)
err = verifyRemoteBranch(sourceProjectName, sourceBranch)
if err != nil {
log.Fatalf("%s:%s\n", sourceRemote, err)
}
Expand All @@ -206,7 +201,7 @@ func runMRCreate(cmd *cobra.Command, args []string) {
if len(args) > 1 && targetBranch != args[1] {
targetBranch = args[1]
// verify the target branch in remote project
err = verifyRemoteBranch(targetProject.ID, targetBranch)
err = verifyRemoteBranch(targetProjectName, targetBranch)
if err != nil {
log.Fatalf("%s:%s\n", targetRemote, err)
}
Expand Down
7 changes: 1 addition & 6 deletions cmd/mr_merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,11 @@ var mrMergeCmd = &cobra.Command{
log.Fatal(err)
}

p, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}

opts := gitlab.AcceptMergeRequestOptions{
MergeWhenPipelineSucceeds: gitlab.Bool(!mergeImmediate),
}

err = lab.MRMerge(p.ID, int(id), &opts)
err = lab.MRMerge(rn, int(id), &opts)
if err != nil {
log.Fatal(err)
}
Expand Down
7 changes: 1 addition & 6 deletions cmd/mr_rebase.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,7 @@ var mrRebaseCmd = &cobra.Command{
log.Fatal(err)
}

p, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}

err = lab.MRRebase(p.ID, int(id))
err = lab.MRRebase(rn, int(id))
if err != nil {
log.Fatal(err)
}
Expand Down
7 changes: 1 addition & 6 deletions cmd/mr_reopen.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,7 @@ var mrReopenCmd = &cobra.Command{
log.Fatal(err)
}

p, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}

err = lab.MRReopen(p.ID, int(id))
err = lab.MRReopen(rn, int(id))
if err != nil {
log.Fatal(err)
}
Expand Down
7 changes: 1 addition & 6 deletions cmd/mr_subscribe.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,7 @@ var mrSubscribeCmd = &cobra.Command{
log.Fatal(err)
}

p, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}

err = lab.MRSubscribe(p.ID, int(id))
err = lab.MRSubscribe(rn, int(id))
if err != nil {
log.Fatal(err)
}
Expand Down
14 changes: 2 additions & 12 deletions cmd/mr_thumb.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,7 @@ var mrThumbUpCmd = &cobra.Command{
log.Fatal(err)
}

p, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}

err = lab.MRThumbUp(p.ID, int(id))
err = lab.MRThumbUp(rn, int(id))
if err != nil {
log.Fatal(err)
}
Expand All @@ -57,12 +52,7 @@ var mrThumbDownCmd = &cobra.Command{
log.Fatal(err)
}

p, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}

err = lab.MRThumbDown(p.ID, int(id))
err = lab.MRThumbDown(rn, int(id))
if err != nil {
log.Fatal(err)
}
Expand Down
7 changes: 1 addition & 6 deletions cmd/mr_unsubscribe.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,7 @@ var mrUnsubscribeCmd = &cobra.Command{
log.Fatal(err)
}

p, err := lab.FindProject(rn)
if err != nil {
log.Fatal(err)
}

err = lab.MRUnsubscribe(p.ID, int(id))
err = lab.MRUnsubscribe(rn, int(id))
if err != nil {
log.Fatal(err)
}
Expand Down
Loading

0 comments on commit 44ebecf

Please sign in to comment.