Update TokenBucket init to respect user TimeSource#4461
Conversation
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
rcoh
left a comment
There was a problem hiding this comment.
At first glance this seems a little weird.
I would have expected no changes other than having the TB read from RuntimeComponents.
Maybe this was the only way to do it?
Discussed with Russell offline and this probably looks like:
I'm out most of the rest of this week, so will probably get to this next week. Moving this PR back to draft until that is done. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Still needs tests, but want to see if it breaks any existing CI first
Motivation and Context
Closes #4459, WASM users were seeing runtime errors because some new functionality added to the
TokenBucketwas creating a defaultTimeSourcethat usedSystemTime. WASM is sandboxed and doesn't have access toSystemTime, so this caused a panic.Description
I updated the
TokenBucketto not carry aTimeSource. Instead the method that need theTimeSourcetake it as an input so the interceptors that use theTokenBucketcan provide it fromRuntimeComponents.Testing
Added integration test to ensure that the
TokenBucketfor an operation picks up theTimeSourcefrom theConfigif it is set.Note: I was going to beef up our WASM integration tests as part of this, but I ran into bytecodealliance/wit-bindgen#1479. So those updates are living in a separate branch for now, you can see the in-progress work here. Added a task to #2499 to update our WASM integration tests to make sure we catch issues like this in the future.
Note2: I still need to clean up that failing exotic platform test. I am using an
AtomicU64which is not available on that platform, but I need to confirm it will be safe to cast tou32.Checklist
.changelogdirectory, specifying "client," "server," or both in theapplies_tokey..changelogdirectory, specifying "aws-sdk-rust" in theapplies_tokey.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.