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.

Sign up for GitHub

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

feat: add ability to configure timeout in AWSClientConfiguration #1152

Merged
merged 8 commits into from
Sep 26, 2023
Merged

feat: add ability to configure timeout in AWSClientConfiguration #1152

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2e99dbf
add ability to configure timeout in AWSClientConfiguration
dayaffe Sep 25, 2023
887b095
configure smithy-swift branch for testing
dayaffe Sep 25, 2023
b8d4c57
revert package.swift change
dayaffe Sep 25, 2023
9c58361
update type of customTimeout in test
dayaffe Sep 25, 2023
7806078
fix typo
dayaffe Sep 26, 2023
b106304
Merge branch 'main' into day/expose-timeout
dayaffe Sep 26, 2023
54a6b1c
fix nit and add test for default values
dayaffe Sep 26, 2023
2ef3729
remove default test, info is not available outside of CRT
dayaffe Sep 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions Sources/Core/AWSClientRuntime/AWSClientConfiguration.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ public class AWSClientConfiguration<ServiceSpecificConfiguration: AWSServiceSpec
/// This structure is custom code-generated for each AWS service.
public var serviceSpecific: ServiceSpecificConfiguration

/// The timeout for a request in milliseconds
///
/// If none is provided the client will use default values based on the operation system.
dayaffe marked this conversation as resolved.
Show resolved Hide resolved
public var connectTimeoutMs: UInt32?

/// Internal designated init
/// All convenience inits should call this.
private init(
Expand All @@ -115,7 +120,8 @@ public class AWSClientConfiguration<ServiceSpecificConfiguration: AWSServiceSpec
_ useFIPS: Swift.Bool?,
_ retryStrategyOptions: RetryStrategyOptions?,
_ appID: String?,
_ awsRetryMode: AWSRetryMode
_ awsRetryMode: AWSRetryMode,
_ connectTimeoutMs: UInt32? = nil
) throws {
typealias RuntimeConfigType =
DefaultSDKRuntimeConfiguration<DefaultRetryStrategy, DefaultRetryErrorInfoProvider>
Expand All @@ -128,8 +134,13 @@ public class AWSClientConfiguration<ServiceSpecificConfiguration: AWSServiceSpec
self.useDualStack = useDualStack
self.useFIPS = useFIPS
self.clientLogMode = RuntimeConfigType.defaultClientLogMode
self.connectTimeoutMs = connectTimeoutMs
self.httpClientConfiguration = RuntimeConfigType.defaultHttpClientConfiguration
self.httpClientEngine = RuntimeConfigType.defaultHttpClientEngine
if let connectTimeoutMs = self.connectTimeoutMs {
self.httpClientEngine = RuntimeConfigType.httpClientEngineWithTimeout(timeoutMs: connectTimeoutMs)
} else {
self.httpClientEngine = RuntimeConfigType.defaultHttpClientEngine
}
self.idempotencyTokenGenerator = RuntimeConfigType.defaultIdempotencyTokenGenerator
self.logger = RuntimeConfigType.defaultLogger(clientName: self.serviceSpecific.clientName)
self.retryStrategyOptions = retryStrategyOptions ?? RuntimeConfigType.defaultRetryStrategyOptions
Expand Down Expand Up @@ -162,7 +173,8 @@ extension AWSClientConfiguration {
useFIPS: Swift.Bool? = nil,
retryMode: AWSRetryMode? = nil,
maxAttempts: Int? = nil,
appID: String? = nil
appID: String? = nil,
connectTimeoutMs: UInt32? = nil
) async throws {
let fileBasedConfig = try await CRTFileBasedConfiguration.makeAsync()
let resolvedRegionResolver = try regionResolver ?? DefaultRegionResolver { _, _ in fileBasedConfig }
Expand Down Expand Up @@ -200,7 +212,8 @@ extension AWSClientConfiguration {
useFIPS,
retryStrategyOptions,
resolvedAppID,
resolvedAWSRetryMode
resolvedAWSRetryMode,
connectTimeoutMs
)
}

Expand All @@ -214,7 +227,8 @@ extension AWSClientConfiguration {
useFIPS: Swift.Bool? = nil,
retryMode: AWSRetryMode? = nil,
maxAttempts: Int? = nil,
appID: String? = nil
appID: String? = nil,
connectTimeoutMs: UInt32? = nil
) throws {
let fileBasedConfig = try CRTFileBasedConfiguration.make()
let resolvedCredentialsProvider: CredentialsProviding
Expand Down Expand Up @@ -245,7 +259,8 @@ extension AWSClientConfiguration {
useFIPS,
retryStrategyOptions,
resolvedAppID,
resolvedAWSRetryMode
resolvedAWSRetryMode,
connectTimeoutMs
)
}

Expand Down
9 changes: 9 additions & 0 deletions Tests/Core/AWSClientRuntimeTests/AWSClientConfigurationTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ class AWSClientConfigurationTests: XCTestCase {
let subject = try await Subject(region: region, appID: appID)
XCTAssertEqual(subject.appID, appID)
}

// MARK: - Timeout

func test_async_configureTimeoutOptionsFromParams() throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we also need a test for default timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding one now. Will check for 30_000 or 3_000 depending on the operating system the test is run on. There doesn't seem to be a way to force the OS in XCTest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this test as default timeout info is in CRT and not available to AWSClientConfiguration

let customTimeout: UInt32 = 10_000
let subject = try Subject(region: region, connectTimeoutMs: customTimeout)
XCTAssertEqual(subject.connectTimeoutMs, customTimeout)
}
}

struct TestAWSServiceSpecificConfiguration: AWSServiceSpecificConfiguration {
Expand All @@ -78,4 +86,5 @@ struct TestAWSServiceSpecificConfiguration: AWSServiceSpecificConfiguration {

var serviceName: String { "TestAWSService" }
var clientName: String { "TestAWSServiceClient" }
var connectTimeoutMs: UInt32 { 10_000 }
}
Loading