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

fix: move smithy config customization into the smithy code generator #1599

Merged
merged 14 commits into from
Aug 16, 2022

Conversation

Velfi
Copy link
Contributor

@Velfi Velfi commented Jul 28, 2022

Motivation and Context

#1598

Description

Previously, the config customizations that added functionality related to retry configs, timeout configs, and the async sleep impl were defined in the smithy codegen module but were being loaded in the AWS codegen module. They have now been updated to be loaded during smithy codegen. The affected classes are all defined in the software.amazon.smithy.rust.codegen.smithy.customizations module of smithy codegen. This change does not affect the generated code.

These classes have been removed:

  • RetryConfigDecorator
  • SleepImplDecorator
  • TimeoutConfigDecorator

These classes have been renamed:

  • RetryConfigProviderConfig is now RetryConfigProviderCustomization
  • PubUseRetryConfig is now PubUseRetryConfigGenerator
  • SleepImplProviderConfig is now SleepImplProviderCustomization
  • TimeoutConfigProviderConfig is now TimeoutConfigProviderCustomization

Testing

Ran existing tests

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates

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

@Velfi Velfi requested a review from a team as a code owner July 28, 2022 18:48
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

github-actions bot commented Aug 8, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@Velfi Velfi self-assigned this Aug 8, 2022
@github-actions
Copy link

github-actions bot commented Aug 8, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

CHANGELOG.next.toml Outdated Show resolved Hide resolved
let retry_config = conf.retry_config.as_ref().cloned().unwrap_or_default();
let timeout_config = conf.timeout_config.as_ref().cloned().unwrap_or_default();
let sleep_impl = conf.sleep_impl.clone();
let retry_config = conf.retry_config().cloned().unwrap_or_default();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking since it was already doing this before: it seems like timeout_config and retry_config could both be pub(crate) and then we could avoid the clones since we already have ownership of the Config struct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't do that b/c we need to use the Config struct to build the service client. Only part of it is used to create the inner smithy client.

Co-authored-by: John DiSanti <[email protected]>
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@Velfi Velfi merged commit e4e9a1d into main Aug 16, 2022
@Velfi Velfi deleted the fix/move-smithy-config-customization-into-smithy-codegen branch August 16, 2022 20:47
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