Skip to content
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

DEVPROD-7219 Remove db access from cli commands #7912

Merged
merged 16 commits into from
May 29, 2024
Merged

DEVPROD-7219 Remove db access from cli commands #7912

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0aa4db6
DEVPROD-7219 Remove db access from cli commands
bynn May 22, 2024
d30c706
use previous value
bynn May 22, 2024
27b8455
nil fix
bynn May 22, 2024
bcb5ebc
add field to graphql'
bynn May 23, 2024
7798b4c
remove unneeded
bynn May 23, 2024
d7f0542
graphql test
bynn May 24, 2024
8a8ff09
rm old
bynn May 24, 2024
d34d61b
2 tests
bynn May 24, 2024
6bdec70
new file'
bynn May 24, 2024
c201387
tets fix
bynn May 24, 2024
2d2b55d
client version
bynn May 24, 2024
a729a60
rename
bynn May 24, 2024
b7485bc
log
bynn May 24, 2024
0a6417c
Merge branch 'main' into DEVPROD-7219
bynn May 24, 2024
3bd1c24
graphql file
bynn May 28, 2024
b15eb00
api patch smaller
bynn May 28, 2024
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
2 changes: 1 addition & 1 deletion config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var (

// ClientVersion is the commandline version string used to control updating
// the CLI. The format is the calendar date (YYYY-MM-DD).
ClientVersion = "2024-05-17"
ClientVersion = "2024-05-22"

// Agent version to control agent rollover. The format is the calendar date
// (YYYY-MM-DD).
Expand Down
2 changes: 1 addition & 1 deletion operations/patch_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ func getJSONPatchDisplay(ac *legacyClient, params outputPatchParams) (string, er
display := []restModel.APIPatch{}
for _, p := range params.patches {
api := restModel.APIPatch{}
err := api.BuildFromService(p, nil)
err := api.BuildFromService(p, &restModel.APIPatchArgs{FromCLI: true})
if err != nil {
return "", errors.Wrap(err, "converting patch to API model")
}
Expand Down
26 changes: 16 additions & 10 deletions rest/model/patch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ type APIPatchArgs struct {
IncludeProjectIdentifier bool
IncludeCommitQueuePosition bool
IncludeChildPatches bool
FromCLI bool
}

// BuildFromService converts from service level structs to an APIPatch.
Expand All @@ -155,24 +156,29 @@ func (apiPatch *APIPatch) BuildFromService(p patch.Patch, args *APIPatchArgs) er
apiPatch.buildBasePatch(p)

projectIdentifier := p.Project
proj, err := model.FindMergedProjectRef(projectIdentifier, p.Version, false)
if err != nil {
return errors.Wrapf(err, "finding project ref '%s'", projectIdentifier)
}
if proj == nil {
return errors.Errorf("project ref '%s' not found", projectIdentifier)
}
// Projects that use the GitHub merge queue cannot enqueue to the commit queue.
apiPatch.CanEnqueueToCommitQueue = (p.HasValidGitInfo() || p.IsGithubPRPatch()) && proj.CommitQueue.MergeQueue != model.MergeQueueGitHub
Copy link
Contributor

@ablack12 ablack12 May 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field is only used by the UI, right? Instead of including this here, could we instead remove CanEnqueueToCommitQueue from the APIPatch struct and instead just add a resolver for it? (like

evergreen/graphql/patch_resolver.go

Line 62 in e002320

func (r *patchResolver) CommitQueuePosition(ctx context.Context, obj *restModel.APIPatch) (*int, error) {
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(@khelif96 is that reasonable?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this makes sense to me


apiPatch.CanEnqueueToCommitQueue = p.HasValidGitInfo() || p.IsGithubPRPatch()
if args != nil {
// CLI cannot access DB information.
if !args.FromCLI {
proj, err := model.FindMergedProjectRef(projectIdentifier, p.Version, false)
if err != nil {
return errors.Wrapf(err, "finding project ref '%s'", projectIdentifier)
}
if proj == nil {
return errors.Errorf("project ref '%s' not found", projectIdentifier)
}
// Projects that use the GitHub merge queue cannot enqueue to the commit queue.
apiPatch.CanEnqueueToCommitQueue = (p.HasValidGitInfo() || p.IsGithubPRPatch()) && proj.CommitQueue.MergeQueue != model.MergeQueueGitHub
}

if args.IncludeProjectIdentifier && p.Project != "" {
apiPatch.GetIdentifier()
if apiPatch.ProjectIdentifier != nil {
projectIdentifier = utility.FromStringPtr(apiPatch.ProjectIdentifier)
}

}

if args.IncludeCommitQueuePosition {
if err := apiPatch.GetCommitQueuePosition(); err != nil {
return errors.Wrap(err, "getting commit queue position")
Expand Down
Loading