Fixed regex for matching size, localhost. Error logging in defer call

[saisrivathsa]

• The regexes used are not valid regular expressions and the test cases pass with the changed regex.
• Error handling while using defer and go commands

[csweichel]

/werft run

👍 started the job as gitpod-build-main-fork.1

[corneliusludmann]

Hi @saisrivathsa,

Thanks for your contribution! Looks good!

Could you do me a favor and consider the following two things:

First: Would you mind squashing/renaming your commits to something more meaningful? I would suggest something like this:

• One commit for the defer close functions with a commit message like Improve error handling in defer function calls
• One commit for the regex changes: Fix regex strings in gitpod-cli and ws-daemon

Second: Would you mind adding the change of my comment to the size.go file?

Thanks!

[corneliusludmann]

While we are at it, I would also suggest the following change:

Suggested change

	sizeRegexp = regexp.MustCompile(`(\d+)(k|m|g|t)?`)
	sizeRegexp = regexp.MustCompile(`^(\d+)(k|m|g|t)?$`)

That would also fix this one:

gitpod/components/ws-daemon/pkg/quota/size_test.go

Lines 26 to 27 in b7dda84

	// This test should fail but doesn't because match somehow matches the regexp
	// {"-42m", 0, quota.ErrInvalidSize},

Would be great if you could then make those changes to the tests as well:

diff --git a/components/ws-daemon/pkg/quota/size_test.go b/components/ws-daemon/pkg/quota/size_test.go
index cc21ae25..68b3b822 100644
--- a/components/ws-daemon/pkg/quota/size_test.go
+++ b/components/ws-daemon/pkg/quota/size_test.go
@@ -23,8 +23,8 @@ func TestParseSize(t *testing.T) {
                {"42m", 42 * quota.Megabyte, nil},
                {"42g", 42 * quota.Gigabyte, nil},
                {"42t", 42 * quota.Terabyte, nil},
-               // This test should fail but doesn't because match somehow matches the regexp
-               // {"-42m", 0, quota.ErrInvalidSize},
+               {"-42m", 0, quota.ErrInvalidSize},
+               {"x42mx", 0, quota.ErrInvalidSize},
                {"abc", 0, quota.ErrInvalidSize},
                {"99999999999999999999999999999999999999999999999999999999999999999999999999999999", 0, quota.ErrInvalidSize},
        }

What do you think?

[gtsiolis]

@saisrivathsa let us know if you up for picking up this and addressing the comments in #3679 (review). You also need to sign a Contributor License Agreement (CLA) before merging this. Cc @meysholdt

[meysholdt]

Hi @saisrivathsa! I've reached out to you via email about the CLA.

[meysholdt]

hi @saisrivathsa ! Thank you for your contribution. Before we can merge it, we'll need to sign a CLA. I've sent you an email about the next steps.

[meysholdt]

hi @saisrivathsa, we've updated our CLA process. Maybe it's easier for you now. Can you please sign our CLA via this DocuSign form? If there are any questions, you can reach me via moritz@gitpod.io.

[gtsiolis]

Closing this for now. 📕

Looping below corresponding teams below if any of the changes here make sense to pick up in separate PRs.
Cc @lucasvaltl + @kylos101 as @gitpod-io/engineering-workspace is code owner for changes here.
Cc @laushinka + @loujaybee as @gitpod-io/engineering-ide is code owner for changes here.