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
ceph: pass region to newS3agent() #8766
Conversation
If the region is specified in the storage class of OBC, use that in the newS3agent() than using constant "us-east-1". Signed-off-by: Jiffin Tony Thottan <thottanjiffin@gmail.com>
func newS3Agent(accessKey, secretKey, endpoint, region string, debug bool, tlsCert []byte, insecure bool) (*S3Agent, error) { | ||
var cephRegion = "us-east-1" | ||
if region != "" { | ||
cephRegion = region |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about a simple unit test that checks the region was applied?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I was not able to find a way to mock s3 client for this unit test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we read the region out of sess
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another approach could be a helper method to get the region, which would be unit tested. While this small of an issue isn't critical for unit testing, in general we really want to have the mindset of unit testing everything within reason. For now though let's get it into the release today.
ceph: pass region to newS3agent() (backport #8766)
Description of your changes:
If the region is specified in the storage class of OBC, use that in the
newS3agent() than using constant "us-east-1".
Signed-off-by: Jiffin Tony Thottan thottanjiffin@gmail.com
Which issue is resolved by this Pull Request:
Resolves #
Checklist:
make codegen
) has been run to update object specifications, if necessary.