Skip to content
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

Env vars and profile to configure IMDS retry and timeouts (#625) #626

Closed
wants to merge 2 commits into from
Closed

Env vars and profile to configure IMDS retry and timeouts (#625) #626

wants to merge 2 commits into from

Conversation

kevinpark1217
Copy link
Contributor

@kevinpark1217 kevinpark1217 commented Oct 2, 2022

Motivation and Context

PR for issue #625

  1. Created Environment struct which handles fetching and parsing of the environment variables and profile values.
  2. Refactored EndpointSource to use the new Environment Struct

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Velfi
Copy link
Contributor

Velfi commented Oct 3, 2022

Hey @kevinpark1217, I'm sorry to say that you've submitted a PR to the wrong repo. You'll need to make this change in smithy-rs instead. Once you re-submit, I have some other comments. Before merging this change, I'm going to ensure that it's actually a standard that we want to support. Internally, the relevant standards doc doesn't reference those environment variables so I'll have to work with the author to get them added.

Also, there is an alternate way to accomplish what you're doing here that I want to propose.

@Velfi Velfi closed this Oct 3, 2022
@kevinpark1217
Copy link
Contributor Author

Internally, the relevant standards doc doesn't reference those environment variables so I'll have to work with the author to get them added.

@Velfi What do you mean by there is no standards doc? Quick Google search brings this up.
https://docs.aws.amazon.com/sdkref/latest/guide/feature-ec2-instance-metadata.html

@Velfi
Copy link
Contributor

Velfi commented Oct 4, 2022

Hey, sorry. The key word was "internally." We have an amazon-only repository of standards that SDKs must implement. While we have some IMDS related standards defined there, non of those docs mention the environment variables specified in the ec2-instance-metadata doc. This means that that doc may be outdated (or that the relevant internal standards docs are.) For this reason, it's important that we figure out which before proceeding.

On another note, the reason that those env vars are proposed in the ec2-instance-metadata doc is because the AWS CLI supports them. The AWS CLI is sort of a special case among SDKs because it can't be configured except through environment variables or AWS profile variables. In the case of the Rust SDK, it's actually possible to programmatically set the IMDS client retry attempts and timeouts, so supporting these vars is not strictly necessary ( unless the internal, non-public standards doc is updated to say so.) I had intended to write up an example demonstrating how once you re-submitted your PR to the smithy-rs repo so we could discuss whether or not to go with your proposed approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants