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.

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

ISSUE: 1585 | Add --wait flag to databases resize command #1590

Conversation

Yordaniss
Copy link
Contributor

@Yordaniss Yordaniss commented Oct 8, 2024

ISSUE #1585

  • added bool flag into resize command
  • added wait functionality into resize
  • adjusted test

* added bool flag into resize command
* added wait functionality into resize
* adjusted test
@Yordaniss Yordaniss changed the title ISSUE: 1585 ISSUE/1585 Oct 8, 2024
@Yordaniss Yordaniss changed the title ISSUE/1585 ISSUES/1585 Oct 8, 2024
@Yordaniss Yordaniss changed the title ISSUES/1585 Close #1585 Oct 8, 2024
@Yordaniss Yordaniss changed the title Close #1585 #1585 Oct 8, 2024
@Yordaniss Yordaniss changed the title #1585 ISSUE: 1585 | Add --wait flag to databases resize command Oct 8, 2024
loosla and others added 4 commits October 8, 2024 13:16
+ clean up with fmt
+ clean up make go fmt
+ removed test option to make it similar to create command
Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

Hi @Yordaniss. Thank you so much for the pull request!

I've left a few comments in-line with suggestions. In addition to that, it would be great to add a new test case to the TestDatabaseResize test.

func TestDatabaseResize(t *testing.T) {

Something along the line of:

	// Success with wait flag
	withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
		tm.databases.EXPECT().Resize(testDBCluster.ID, r).Return(nil)
		tm.databases.EXPECT().Get(testDBCluster.ID).Return(&testDBCluster, nil)
		config.Args = append(config.Args, testDBCluster.ID)
		config.Doit.Set(config.NS, doctl.ArgSizeSlug, testDBCluster.SizeSlug)
		config.Doit.Set(config.NS, doctl.ArgDatabaseNumNodes, testDBCluster.NumNodes)
		config.Doit.Set(config.NS, doctl.ArgDatabaseStorageSizeMib, testDBCluster.StorageSizeMib)
		config.Doit.Set(config.NS, doctl.ArgCommandWait, true)

		err := RunDatabaseResize(config)
		assert.NoError(t, err)
	})

Comparing that to the existing tests, you'll see it passes the wait flag and tests that the Get method is called by waitForDatabaseReady when the flag is passed.

Let us know if you have any questions!

commands/databases.go Outdated Show resolved Hide resolved
commands/databases.go Outdated Show resolved Hide resolved
+ added new test case with wait flag
+ simplify logic in function
@Yordaniss
Copy link
Contributor Author

Hi @andrewsomething thanks a lot for a feedback. I simplified logic and added test case. 🙂

@andrewsomething andrewsomething self-requested a review October 9, 2024 19:48
Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

👍 Great work! Thanks again for the contribution!

@andrewsomething andrewsomething merged commit 0558237 into digitalocean:main Oct 9, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants