8000 fix(engine): do not load all branches all the time by sguiheux · Pull Request #5900 · ovh/cds · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(engine): do not load all branches all the time #5900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 30, 2021
Merged

fix(engine): do not load all branches all the time #5900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
61077a7
fix(engine): do not load all branches all the time
sguiheux Aug 5, 2021
ce6782a
fix: remove logs
sguiheux Aug 5, 2021
7f98ca2
fix: unit test mock vcs route
sguiheux Aug 5, 2021
1998352
fix: unit test
sguiheux Aug 5, 2021
8a2dc9c
fix: ut
sguiheux Aug 5, 2021
c375b41
fix: ut
sguiheux Aug 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions engine/api/application.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func (api *API) getApplicationVCSInfosHandler() service.Handler {
if remote != "" && remote != app.RepositoryFullname {
repositoryFullname = remote
}
branches, err := client.Branches(ctx, repositoryFullname)
branches, err := client.Branches(ctx, repositoryFullname, sdk.VCSBranchesFilter{Limit: 50})
if err != nil {
return err
}
Expand Down Expand Up @@ -481,7 +481,7 @@ func (api *API) updateAsCodeApplicationHandler() service.Handler {
return sdk.WrapError(sdk.ErrNoReposManagerClientAuth, "updateAsCodeApplicationHandler> Cannot get client got %s %s : %v", key, appDB.VCSServer, err)
}

b, err := client.Branch(ctx, appDB.RepositoryFullname, branch)
b, err := client.Branch(ctx, appDB.RepositoryFullname, sdk.VCSBranchFilters{BranchName: branch})
if err != nil && !sdk.ErrorIs(err, sdk.ErrNotFound) {
return err
}
Expand Down
12 changes: 4 additions & 8 deletions engine/api/application_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,29 +134,25 @@ func TestUpdateAsCodeApplicationHandler(t *testing.T) {
return nil, 200, nil
},
)
servicesClients.EXPECT().DoJSONRequest(gomock.Any(), "GET", "/vcs/github/repos/foo/myrepo/branches", gomock.Any(), gomock.Any(), gomock.Any()).
servicesClients.EXPECT().DoJSONRequest(gomock.Any(), "GET", "/vcs/github/repos/foo/myrepo/branches/?branch=&default=true", gomock.Any(), gomock.Any(), gomock.Any()).
DoAndReturn(func(ctx context.Context, method, path string, in interface{}, out interface{}, _ interface{}) (http.Header, int, error) {
bs := []sdk.VCSBranch{}
b := sdk.VCSBranch{
DisplayID: "master",
LatestCommit: "aaaaaaa",
}
bs = append(bs, b)
out = bs
out = b
return nil, 200, nil
}).Times(1)

servicesClients.EXPECT().
DoJSONRequest(gomock.Any(), "GET", "/vcs/github/repos/foo/myrepo/branches/?branch=master", gomock.Any(), gomock.Any(), gomock.Any()).
DoJSONRequest(gomock.Any(), "GET", "/vcs/github/repos/foo/myrepo/branches/?branch=master&default=false", gomock.Any(), gomock.Any(), gomock.Any()).
DoAndReturn(
func(ctx context.Context, method, path string, in interface{}, out interface{}, _ interface{}) (http.Header, int, error) {
bs := []sdk.VCSBranch{}
b := sdk.VCSBranch{
DisplayID: "master",
Default: false,
}
bs = append(bs, b)
out = bs
out = b
return nil, 404, nil
},
).MaxTimes(1)
Expand Down
9 changes: 2 additions & 7 deletions engine/api/ascode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,11 @@ func (api *API) postImportAsCodeHandler() service.Handler {
sdk.NewErrorFrom(sdk.ErrNoReposManagerClientAuth, "cannot get client for %s %s", key, ope.VCSServer))
}

branches, err := client.Branches(ctx, ope.RepoFullName)
branch, err := repositoriesmanager.DefaultBranch(ctx, client, ope.RepoFullName)
if err != nil {
return err
}
for _, b := range branches {
if b.Default {
ope.Setup.Checkout.Branch = b.DisplayID
break
}
}
ope.Setup.Checkout.Branch = branch.DisplayID

if err := operation.PostRepositoryOperation(ctx, tx, *p, ope, nil); err != nil {
return sdk.WrapError(err, "cannot create repository operation")
Expand Down
6 changes: 2 additions & 4 deletions engine/api/ascode_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,11 @@ func Test_postImportAsCodeHandler(t *testing.T) {
w.Body = ioutil.NopCloser(body)

switch r.URL.String() {
case "/vcs/github/repos/myrepo/branches":
bs := []sdk.VCSBranch{}
case "/vcs/github/repos/myrepo/branches/?branch=&default=true":
b := sdk.VCSBranch{
DisplayID: "master",
}
bs = append(bs, b)
if err := enc.Encode(bs); err != nil {
if err := enc.Encode(b); err != nil {
return writeError(w, err)
}
default:
Expand Down
2 changes: 1 addition & 1 deletion engine/api/environment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ func (api *API) updateAsCodeEnvironmentHandler() service.Handler {
return sdk.WrapError(sdk.ErrNoReposManagerClientAuth, "updateAsCodeEnvironmentHandler> Cannot get client got %s %s : %v", key, rootApp.VCSServer, err)
}

b, err := client.Branch(ctx, rootApp.RepositoryFullname, branch)
b, err := client.Branch(ctx, rootApp.RepositoryFullname, sdk.VCSBranchFilters{BranchName: branch})
if err != nil && !sdk.ErrorIs(err, sdk.ErrNotFound) {
return err
}
Expand Down
12 changes: 4 additions & 8 deletions engine/api/environment_ascode_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,29 +87,25 @@ func TestUpdateAsCodeEnvironmentHandler(t *testing.T) {
return nil, 200, nil
},
)
servicesClients.EXPECT().DoJSONRequest(gomock.Any(), "GET", "/vcs/github/repos/foo/myrepo/branches", gomock.Any(), gomock.Any(), gomock.Any()).
servicesClients.EXPECT().DoJSONRequest(gomock.Any(), "GET", "/vcs/github/repos/foo/myrepo/branches/?branch=&default=true", gomock.Any(), gomock.Any(), gomock.Any()).
DoAndReturn(func(ctx context.Context, method, path string, in interface{}, out interface{}, _ interface{}) (http.Header, int, error) {
bs := []sdk.VCSBranch{}
b := sdk.VCSBranch{
DisplayID: "master",
LatestCommit: "aaaaaaa",
}
bs = append(bs, b)
out = bs
out = b
return nil, 200, nil
}).Times(1)

servicesClients.EXPECT().
DoJSONRequest(gomock.Any(), "GET", "/vcs/github/repos/foo/myrepo/branches/?branch=master", gomock.Any(), gomock.Any(), gomock.Any()).
DoJSONRequest(gomock.Any(), "GET", "/vcs/github/repos/foo/myrepo/branches/?branch=master&default=false", gomock.Any(), gomock.Any(), gomock.Any()).
DoAndReturn(
func(ctx context.Context, method, path string, in interface{}, out interface{}, _ interface{}) (http.Header, int, error) {
bs := []sdk.VCSBranch{}
b := sdk.VCSBranch{
DisplayID: "master",
Default: false,
}
bs = append(bs, b)
out = bs
out = b
return nil, 404, nil
},
).MaxTimes(1)
Expand Down
2 changes: 1 addition & 1 deletion engine/api/pipeline.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (api *API) updateAsCodePipelineHandler() service.Handler {
return sdk.WrapError(sdk.ErrNoReposManagerClientAuth, "updateAsCodePipelineHandler> cannot get client got %s %s : %v", key, rootApp.VCSServer, err)
}

b, err := client.Branch(ctx, rootApp.RepositoryFullname, branch)
b, err := client.Branch(ctx, rootApp.RepositoryFullname, sdk.VCSBranchFilters{BranchName: branch})
if err != nil && !sdk.ErrorIs(err, sdk.ErrNotFound) {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion engine/api/purge/purge_run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func ApplyRetentionPolicyOnWorkflow(ctx context.Context, store cache.Store, db *

branchesMap := make(map[string]struct{})
if vcsClient != nil {
branches, err := vcsClient.Branches(ctx, app.RepositoryFullname)
branches, err := vcsClient.Branches(ctx, app.RepositoryFullname, sdk.VCSBranchesFilter{})
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions engine/api/repositories_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestAPI_detachRepositoriesManagerHandler(t *testing.T) {
return writeError(w, err)
}
// Default payload on workflow insert
case "/vcs/github/repos/sguiheux/demo/branches":
case "/vcs/github/repos/sguiheux/demo/branches/?branch=&default=true":
b := sdk.VCSBranch{
Default: true,
DisplayID: "master",
Expand All @@ -79,7 +79,7 @@ func TestAPI_detachRepositoriesManagerHandler(t *testing.T) {
return writeError(w, err)
}
// NEED GET BRANCH TO GET LATEST COMMIT
case "/vcs/github/repos/sguiheux/demo/branches/?branch=master":
case "/vcs/github/repos/sguiheux/demo/branches/?branch=master&default=false":
b := sdk.VCSBranch{
Default: false,
DisplayID: "master",
Expand Down
19 changes: 7 additions & 12 deletions engine/api/repositoriesmanager/repositories_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,14 +335,14 @@ func (c *vcsClient) Tags(ctx context.Context, fullname string) ([]sdk.VCSTag, er
return tags, nil
}

func (c *vcsClient) Branches(ctx context.Context, fullname string) ([]sdk.VCSBranch, error) {
func (c *vcsClient) Branches(ctx context.Context, fullname string, filters sdk.VCSBranchesFilter) ([]sdk.VCSBranch, error) {
items, has := c.Cache().Get("/branches/" + fullname)
if has {
return items.([]sdk.VCSBranch), nil
}

branches := []sdk.VCSBranch{}
path := fmt.Sprintf("/vcs/%s/repos/%s/branches", c.name, fullname)
pat CEB7 h := fmt.Sprintf("/vcs/%s/repos/%s/branches?limit=%d", c.name, fullname, filters.Limit)
if _, err := c.doJSONRequest(ctx, "GET", path, nil, &branches); err != nil {
return nil, sdk.NewErrorFrom(err, "unable to find branches on repository %s from %s", fullname, c.name)
}
Expand All @@ -352,27 +352,22 @@ func (c *vcsClient) Branches(ctx context.Context, fullname string) ([]sdk.VCSBra
return branches, nil
}

func (c *vcsClient) Branch(ctx context.Context, fullname string, branchName string) (*sdk.VCSBranch, error) {
func (c *vcsClient) Branch(ctx context.Context, fullname string, filters sdk.VCSBranchFilters) (*sdk.VCSBranch, error) {
branch := sdk.VCSBranch{}
path := fmt.Sprintf("/vcs/%s/repos/%s/branches/?branch=%s", c.name, fullname, url.QueryEscape(branchName))
path := fmt.Sprintf("/vcs/%s/repos/%s/branches/?branch=%s&default=%v", c.name, fullname, url.QueryEscape(filters.BranchName), filters.Default)
if _, err := c.doJSONRequest(ctx, "GET", path, nil, &branch); err != nil {
return nil, sdk.WrapError(err, "unable to find branch %s on repository %s from %s", branchName, fullname, c.name)
return nil, sdk.WrapError(err, "unable to find branch %s/%v on repository %s from %s", filters.BranchName, filters.Default, fullname, c.name)
}
return &branch, nil
}

// DefaultBranch get default branch from given repository
func DefaultBranch(ctx context.Context, c sdk.VCSAuthorizedClientCommon, fullname string) (sdk.VCSBranch, error) {
branches, err := c.Branches(ctx, fullname)
branch, err := c.Branch(ctx, fullname, sdk.VCSBranchFilters{Default: true})
if err != nil {
return sdk.VCSBranch{}, err
}
for _, b := range branches {
if b.Default {
return b, nil
}
}
return sdk.VCSBranch{}, sdk.NewErrorFrom(sdk.ErrNotFound, "unable to find default branch on repository %s (among %d branches)", fullname, len(branches))
return *branch, nil
}

func (c *vcsClient) Commits(ctx context.Context, fullname, branch, since, until string) ([]sdk.VCSCommit, error) {
Expand Down
9 changes: 2 additions & 7 deletions engine/api/workflow/dao_coverage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,16 +180,11 @@ func ComputeLatestDefaultBranchReport(ctx context.Context, db gorpmapper.SqlExec
return sdk.WrapError(sdk.ErrNoReposManagerClientAuth, "ComputeLatestDefaultBranchReport> Cannot get repo client %s : %s", wnr.VCSServer, erra)
}

branches, err := client.Branches(ctx, wnr.VCSRepository)
branch, err := repositoriesmanager.DefaultBranch(ctx, client, wnr.VCSRepository)
if err != nil {
return err
}
for _, b := range branches {
if b.Default {
defaultBranch = b.DisplayID
break
}
}
defaultBranch = branch.DisplayID

if defaultBranch != wnr.VCSBranch {
defaultCoverage, errD := loadLatestCoverageReport(db, wnr.WorkflowID, wnr.VCSRepository, defaultBranch, covReport.ApplicationID)
Expand Down
18 changes: 4 additions & 14 deletions engine/api/workflow/dao_node_run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,21 +596,11 @@ func GetNodeRunBuildCommits(ctx context.Context, db gorpmapper.SqlExecutorWithTx
}

if cur.Branch == "" && cur.Tag == "" {
branches, err := client.Branches(ctx, cur.Remote)
branch, err := repositoriesmanager.DefaultBranch(ctx, client, cur.Remote)
if err != nil {
return nil, cur, sdk.WrapError(err, "cannot load branches from vcs api remote %s", cur.Remote)
}
found := false
for _, br := range branches {
if br.Default {
cur.Branch = br.DisplayID
found = true
break
}
}
if !found {
return nil, cur, sdk.WithStack(fmt.Errorf("cannot find default branch from vcs api"))
}
cur.Branch = branch.DisplayID
}

var envID int64
Expand All @@ -626,7 +616,7 @@ func GetNodeRunBuildCommits(ctx context.Context, db gorpmapper.SqlExecutorWithTx
var lastCommit sdk.VCSCommit
if cur.Hash == "" && cur.Tag == "" && cur.Branch != "" {
//If we only have the current branch, search for the branch
br, err := client.Branch(ctx, repo, cur.Branch)
br, err := client.Branch(ctx, repo, sdk.VCSBranchFilters{BranchName: cur.Branch})
if err != nil {
return nil, cur, sdk.WrapError(err, "cannot get branch %s", cur.Branch)
}
Expand Down Expand Up @@ -660,7 +650,7 @@ func GetNodeRunBuildCommits(ctx context.Context, db gorpmapper.SqlExecutorWithTx
} else if prev.Hash != "" {
if cur.Tag == "" {
if cur.Hash == "" {
br, err := client.Branch(ctx, repo, cur.Branch)
br, err := client.Branch(ctx, repo, sdk.VCSBranchFilters{BranchName: cur.Branch})
if err != nil {
return nil, cur, sdk.WrapError(err, "Cannot get branch %s", cur.Branch)
}
Expand Down
18 changes: 8 additions & 10 deletions engine/api/workflow/dao_run_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func TestPurgeWorkflowRun(t *testing.T) {
return writeError(w, err)
}
// Default payload on workflow insert
case "/vcs/github/repos/sguiheux/demo/branches":
case "/vcs/github/repos/sguiheux/demo/branches/?branch=&default=true":
b := sdk.VCSBranch{
Default: true,
DisplayID: "master",
Expand All @@ -121,7 +121,7 @@ func TestPurgeWorkflowRun(t *testing.T) {
return writeError(w, err)
}
< 10000 span class='blob-code-inner blob-code-marker ' data-code-marker=" "> // NEED GET BRANCH TO GET LATEST COMMIT
case "/vcs/github/repos/sguiheux/demo/branches/?branch=master":
case "/vcs/github/repos/sguiheux/demo/branches/?branch=master&default=false":
b := sdk.VCSBranch{
Default: false,
DisplayID: "master",
Expand Down Expand Up @@ -362,12 +362,10 @@ func TestPurgeWorkflowRunWithOneSuccessWorkflowRun(t *testing.T) {

switch r.URL.String() {
// NEED get REPO
case "/vcs/github/repos/sguiheux/demo/branches":
branches := []sdk.VCSBranch{
{
ID: "master",
DisplayID: "master",
},
case "/vcs/github/repos/sguiheux/demo/branches/?branch=&default=true":
branches := sdk.VCSBranch{
ID: "master",
DisplayID: "master",
}
if err := enc.Encode(branches); err != nil {
return writeError(w, err)
Expand All @@ -387,7 +385,7 @@ func TestPurgeWorkflowRunWithOneSuccessWorkflowRun(t *testing.T) {
return writeError(w, err)
}
// NEED GET BRANCH TO GET LATEST COMMIT
case "/vcs/github/repos/sguiheux/demo/branches/?branch=master":
case "/vcs/github/repos/sguiheux/demo/branches/?branch=master&default=false":
b := sdk.VCSBranch{
Default: false,
DisplayID: "master",
Expand Down Expand Up @@ -578,7 +576,7 @@ func TestPurgeWorkflowRunWithNoSuccessWorkflowRun(t *testing.T) {
return writeError(w, err)
}
// NEED GET BRANCH TO GET LATEST COMMIT
case "/vcs/github/repos/sguiheux/demo/branches/?branch=master":
case "/vcs/github/repos/sguiheux/demo/branches/?branch=master&default=false":
b := sdk.VCSBranch{
Default: false,
DisplayID: "master",
Expand Down
4 changes: 2 additions & 2 deletions engine/api/workflow/dao_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1660,13 +1660,13 @@ func TestInsertAndDeleteMultiHook(t *testing.T) {
}

// NEED for default payload on insert
case "/vcs/github/repos/sguiheux/demo/branches":
case "/vcs/github/repos/sguiheux/demo/branches/?branch=&default=true":
b := sdk.VCSBranch{
Default: true,
DisplayID: "master",
LatestCommit: "mylastcommit",
}
if err := enc.Encode([]sdk.VCSBranch{b}); err != nil {
if err := enc.Encode(b); err != nil {
return writeError(w, err)
}
case "/task/bulk":
Expand Down
2 changes: 1 addition & 1 deletion engine/api/workflow/execute_node_run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1144,7 +1144,7 @@ func getVCSInfos(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cac
vcsInfos.Hash = defaultB.LatestCommit
case vcsInfos.Hash == "" && vcsInfos.Branch != "":
// GET COMMIT INFO
branch, errB := client.Branch(ctx, vcsInfos.Repository, vcsInfos.Branch)
branch, errB := client.Branch(ctx, vcsInfos.Repository, sdk.VCSBranchFilters{BranchName: vcsInfos.Branch})
if errB != nil {
// Try default branch
b, errD := repositoriesmanager.DefaultBranch(ctx, client, vcsInfos.Repository)
Expand Down
10 changes: 2 additions & 8 deletions engine/api/workflow/hook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,17 +424,11 @@ func DefaultPayload(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store
return wf.WorkflowData.Node.Context.DefaultPayload, sdk.WrapError(err, "cannot get authorized client")
}

branches, err := client.Branches(ctx, app.RepositoryFullname)
branch, err := repositoriesmanager.DefaultBranch(ctx, client, app.RepositoryFullname)
if err != nil {
return wf.WorkflowData.Node.Context.DefaultPayload, err
}

for _, branch := range branches {
if branch.Default {
defaultBranch = branch.DisplayID
break
}
}
defaultBranch = branch.DisplayID
}

defaultPayload = wf.WorkflowData.Node.Context.DefaultPayload
Expand Down
Loading
0