Improved Error Msg For Bucket Names that Contain non-LOWERCASE chars (#2134) #2135
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2135 +/- ##
======================================
Coverage 8.51% 8.51%
======================================
Files 91 91
Lines 6809 6809
======================================
Hits 580 580
Misses 6094 6094
Partials 135 135
Continue to review full report at Codecov.
|
|
@harshavardhana Why did my PR fail the |
|
codecov/patch failure can be ignored for |
|
Ah... ok...
But I didn't add any paths either... just changed a message... :)
…On Thu, Apr 20, 2017 at 9:47 AM, Harshavardhana ***@***.***> wrote:
codecov/patch failure can be ignored for mc for now. We need to up the
coverage on each PR. What it is says that you haven't added anything to
cover the changes which were submitted in this PR.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2135 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADyRnzJXXb2MNSVBerNLglM_Qs7Wg1Hvks5rx4wkgaJpZM4NCI43>
.
|
If you want full green add a test to check for the change in the message. |
cmd/client-s3.go
Outdated
| @@ -746,7 +746,7 @@ func isValidBucketName(bucketName string) *probe.Error { | |||
| return probe.NewError(errors.New("Bucket name should be more than 3 characters and less than 64 characters")) | |||
| } | |||
| if !validBucketName.MatchString(bucketName) { | |||
| return probe.NewError(errors.New("Bucket name can contain alphabet, '-' and numbers, but first character should be an alphabet or number")) | |||
| return probe.NewError(errors.New("Bucket names can contain lowercase alpha characters `a-z`, numbers '0-9', and '-'. First character cannot be a '-'")) | |||
There was a problem hiding this comment.
Isn't Bucket names can *only* contain more firm ?
There was a problem hiding this comment.
I agree with this wording change... will do it now! When I was playing with it yesterday I had the same basic idea...
Improved the error message for various
mccommands where the user specifies a path or url and the bucket name in that path contains non-lowercase characters.More specifically, this applies when the path refers to a backend that is an S3 object store and is going through the S3-client. If the backend is a normal/local file system that follows a different path.
The change is in the function isValidBucketName in cmd/client-s3.go. This routine is invoked when validating the path for
mbcommand (as noted in the problem description) but is also used to validate the bucket name for othermccommands (eg:mc events).Examples:
Scotts-MacBook-Pro-2:mc scott$ mc mb myminio/GoPro
mc: Unable to make bucket
myminio/GoPro. Bucket names can contain lowercase alpha charactersa-z, numbers '0-9', and '-'. First character cannot be a '-'Scotts-MacBook-Pro-2:mc scott$ mc events list myminio/GoPro
mc: Cannot enable notification on the specified bucket. Bucket names can contain lowercase alpha characters
a-z, numbers '0-9', and '-'. First character cannot be a '-'Scotts-MacBook-Pro-2:mc scott$
Note: some
mccommands (such aslsandshare) will report other errors in this same case... We may need to look at these cases as well for consistency.Scotts-MacBook-Pro-2:mc scott$ mc ls myminio/GoPro
mc: Unable to stat
myminio/GoPro. Bucket name contains invalid characters.Scotts-MacBook-Pro-2:mc scott$ mc share download myminio/GoPro
mc: Unable to stat
myminio/GoPro. Bucket name contains invalid characters.Scotts-MacBook-Pro-2:mc scott$