Skip to content

Conversation

@andreaangiolillo
Copy link
Collaborator

@andreaangiolillo andreaangiolillo commented Dec 2, 2022

Proposed changes

Jira ticket: CLOUDP-148788

Checklist

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works
  • I have added any necessary documentation in document requirements section listed in CONTRIBUTING.md (if appropriate)
  • I have addressed the @mongodb/docs-cloud-team comments (if appropriate)
  • I have updated test/README.md (if an e2e test has been added)
  • I have run make fmt and formatted my code

Further comments

This came out in https://jira.mongodb.org/browse/HELP-40211. The cutover command is passing the orgId instead of the projectId

@andreaangiolillo andreaangiolillo marked this pull request as ready for review December 2, 2022 16:59
@andreaangiolillo andreaangiolillo requested a review from a team December 2, 2022 16:59
gssbzn
gssbzn previously approved these changes Dec 2, 2022
Copy link
Contributor

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

LGTM

blva
blva previously approved these changes Dec 2, 2022
PreRunE: func(cmd *cobra.Command, args []string) error {
return opts.PreRunE(
opts.ValidateOrgID,
opts.ValidateProjectID,
Copy link
Collaborator

@colm-quinn colm-quinn Dec 2, 2022

Choose a reason for hiding this comment

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

Well done here on the prompt reply.

Not blocking: Can you do a quick audit of the other LiveMigration commands to make sure they are using the correct parameter? Patterns like this tend to happen in multiple places easily, so would be good to confirm in case it's in other places.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

colm-quinn
colm-quinn previously approved these changes Dec 2, 2022
Copy link
Collaborator

@colm-quinn colm-quinn left a comment

Choose a reason for hiding this comment

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

LGTM

@andreaangiolillo andreaangiolillo dismissed stale reviews from colm-quinn, blva, and gssbzn via edb910a December 5, 2022 09:49
@andreaangiolillo andreaangiolillo requested a review from a team as a code owner December 5, 2022 09:49

func (opts *LiveMigrationsOpts) GenerateFlags(cmd *cobra.Command) {
cmd.Flags().StringVar(&opts.OrgID, flag.OrgID, "", usage.OrgID)
cmd.Flags().StringVar(&opts.ProjectID, flag.ProjectID, "", usage.ProjectID)
Copy link
Contributor

Choose a reason for hiding this comment

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

line 128, there's a built in feature now for that can you migrate to it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

@andreaangiolillo andreaangiolillo merged commit 0acfb08 into master Dec 5, 2022
@andreaangiolillo andreaangiolillo deleted the CLOUDP-148788 branch December 5, 2022 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants