Correctly aquire information on AWS_REGION #49
Conversation
|
Hi! Thank you for this fix! It looks like that your PR has conflicts with the master branch, please rebase |
Hi! Fixed |
| if region, ok := settings[RegionSetting]; ok { | ||
| return region, nil | ||
| } | ||
|
|
||
| if config.Endpoint == nil || |
There was a problem hiding this comment.
This new extracting logic deserves extracting into a separate function, just like the findBucketRegion
|
|
||
| host, _, err := net.SplitHostPort(hostAddr.Host) | ||
|
|
||
| if err != nil { |
There was a problem hiding this comment.
In this logic, if WAL-G failed to parse the endpoint host, it won't attempt to call findBucketRegion, is it intended behavior?
I think that WAL-G should always try both approaches before giving up.
| var config = defaults.Get().Config.WithRegion(settings[RegionSetting]) | ||
|
|
||
| func TestGetAWSRegionWithEmptyEndpoint(t *testing.T) { | ||
| findBucketRegion = func(bucket string, config *aws.Config) (string, error) { |
There was a problem hiding this comment.
Maybe I missed the point, but I think that we just need to provide different aws.Config and compare the result with the expected one? Basically, write unit tests for these functions:
findBucketRegion- The new function which extracts the region from the hostname
|
Hi! This repository is going to be merged into the wal-g main repo. Please submit any future updates to the main repository. Thanks! |
wal-g/wal-g#128