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

Retry/timeout client API ergonomics and consistency #1601

Closed
7 tasks done
jdisanti opened this issue Jul 28, 2022 · 1 comment
Closed
7 tasks done

Retry/timeout client API ergonomics and consistency #1601

jdisanti opened this issue Jul 28, 2022 · 1 comment
Labels
breaking-change This will require a breaking change ergonomics
Milestone

Comments

@jdisanti
Copy link
Collaborator

jdisanti commented Jul 28, 2022

Some notes from playing around with manually configuring timeouts and retries with aws-config:

  • aws_smithy_client::Builder's set_* vs non-set_* functions are inconsistent. The sleep_impl doesn't have a set_ prefix, but the retry and timeout configs do (and don't have a non-set_ prefix).
  • Retry has aws_config::RetryConfig, but timeouts have aws::timeout::Config
  • Timeout config should be re-exported along side with RetryConfig in the generated crate
    • This re-export should happen for both Smithy and AWS clients
  • RetryConfig should be re-exported for both Smithy and AWS clients (currently only exported for AWS clients)
  • Configuring timeouts requires importing TriState from aws_smithy_types. It should be re-exported if it really is required for timeout configuration
  • The TriState for configuring timeouts is a little obtuse. We should follow the consistent pattern of thing/set_thing that the rest of the SDK uses where thing gives thing a value, and set_thing allows for removing that value (or directly setting the TriState in this case).
@jdisanti jdisanti changed the title Retry/timeout API ergonomics and consistency in aws-config Retry/timeout client API ergonomics and consistency Sep 2, 2022
@jdisanti jdisanti added the breaking-change This will require a breaking change label Sep 2, 2022
@Velfi Velfi added this to the SDK GA milestone Sep 26, 2022
@jdisanti
Copy link
Collaborator Author

All these issues were resolved in #1740.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This will require a breaking change ergonomics
Projects
None yet
Development

No branches or pull requests

2 participants