add validation before creating an s3 bucket for scenario testing #12013
Conversation
| except ClientError as exc: | ||
| if exc.response["Error"]["Message"] != "Not Found": | ||
| raise exc |
There was a problem hiding this comment.
| except ClientError as exc: | |
| if exc.response["Error"]["Message"] != "Not Found": | |
| raise exc | |
| except ClientError: | |
| if exc.response["Error"]["Message"] != "Not Found": | |
| raise |
Bit of a nit, feel free to ignore. Just a bit cleaner that way since you don't need the reference
There was a problem hiding this comment.
suggestion: Given the docs on HeadBucket, it might also be a bit cleaner to check the response code (404) instead
If the bucket does not exist or you do not have permission to
access it, the HEAD request returns a generic 400 Bad Request ,
403 Forbidden or 404 Not Found code. A message body is not in-
cluded, so you cannot determine the exception beyond these HTTP
response codes.
There was a problem hiding this comment.
I think you need to keep the reference to exc to be able to check on the status code or error message of it though! But just calling raise is cleaner 👍
Agreed for the status code, HeadBucket is weird and the error message often can come from the client itself, so it's easier to check it.
Personally, I would have done a call to CreateBucket and then suppressed the exceptions if the bucket already existed, so you only need to do one call, but that works too!
d6529fd to
4282d60
Compare
Motivation
As discussed with @dominikschubert @bentsku S3 behaves differently when creating a Bucket in different regions. Being Idempotent when using region
us-east-1and not idempotent in other Regions. This AWS behavior is causing issues with the Multi-region tests. /cc @alexrashedChanges
Testing