Add support for configured endpoints. #2328
Conversation
|
|
||
| // GetServiceBaseEndpoint is used to retrieve a normalized SDK ID for use | ||
| // with configured endpoints. | ||
| func (c EnvConfig) GetServiceBaseEndpoint(ctx context.Context, sdkID string) (string, bool, error) { |
There was a problem hiding this comment.
[nonblocking] In new "resolvers" like this I've just left off the error return if it can't actually return an error.
| @@ -482,6 +494,34 @@ func (c EnvConfig) getCustomCABundle(context.Context) (io.Reader, bool, error) { | |||
| return bytes.NewReader(b), true, nil | |||
| } | |||
|
|
|||
| // GetIgnoreConfiguredEndpoints is used in knowing when to disable configured | |||
| // endpoints feature. | |||
| func (c EnvConfig) GetIgnoreConfiguredEndpoints(context.Context) (bool, bool, error) { | |||
There was a problem hiding this comment.
Why context.Context? Don't see it used anywhere, it's also not possible for the caller to propagate it with the structure of how these are used (I see you're setting it to context.Background() later on).
There was a problem hiding this comment.
Oh I see it's part of that static resolver signature. I would just not propagate it further down wherever it's not needed.
There was a problem hiding this comment.
sure, but in this impl in env config and shared config, i dont propagate it down to any further functions. feel free to elaborate if im misunderstanding
isaiahvita
force-pushed
the
endpoint-config
branch
3 times, most recently
from
4dc8ebb
to
76c041d
Compare
isaiahvita
force-pushed
the
endpoint-config
branch
from
54bcc53
to
9b5268a
Compare
.github/workflows/go.yml
Outdated
| @@ -21,7 +21,7 @@ jobs: | |||
| strategy: | |||
| matrix: | |||
| os: [ubuntu-latest, macos-latest] | |||
| go-version: [1.19, "1.20"] | |||
| go-version: ["1.20", "1.21"] | |||
There was a problem hiding this comment.
Why are we doing this here?
There was a problem hiding this comment.
this was before i root caused my issue. and thought this might fix it. but i left it because 1.21 is the latest released version. all tests were passing with it, but i can change it back if you think we should
There was a problem hiding this comment.
Yeah revert this. We technically still need to check 1.19 and I'm going to handle this in a separate PR today anyway.
isaiahvita
force-pushed
the
endpoint-config
branch
from
94d9a6f
to
e04f8fa
Compare
isaiahvita
force-pushed
the
endpoint-config
branch
from
e04f8fa
to
c9fba92
Compare
While SDK consumers can already configure endpoint overrides in code, consumers have expressed the desire for the same configuration options in environment variables and in the aws config file. This PR fulfills this feature.
This PR regenerated code for all services
In order to support this feature, support need to be added for subproperties in the aws config file. It was decided to extend the INI lexer to treat subproperties as literals. Then parse these literals in the config parser.
It was specifically avoided to add this support in the INI parser. This is because the current state of the INI parser:
Feature context: aws/aws-sdk#229
Depends on: aws/smithy-go#463