-
Notifications
You must be signed in to change notification settings - Fork 190
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: time not implemented on wasm #2262
Conversation
I think we will probably want to figure out a way to do this without pulling in the wasm-bindgen — My guess is we'll want to refactor https://github.com/awslabs/smithy-rs/blob/main/aws/rust-runtime/aws-credential-types/src/time_source.rs into a dyn-safe trait like we have to Probably worth an RFC to get everyone aligned on the design |
I agree with you with pulling I don't think we want to every time we need to fetch the current time to always go through the TimeSource (that was passed during instantiation or default), do you? I think this will somewhat become a burden while developing it. In the ideal scenario, we would want the standard library itself to not throw this error. This might happen when there is a standard way of dealing with of the host environment in a WebAssembly context (possibly WebAssembly component model will solve this). |
@rcoh , I converted the PR back to draft. Let me know if you still think it is worth going on the route of this PR, otherwise I will park this for now. |
I had a discussion with @rcoh this morning about this. We want to take the following approach:
You're welcome to implement this if you want, and I can provide more details and answer questions if you're interested. On the fallibility of |
Based on this comment from the wasm-timer maintainer, the approach you guys are proposing seems a good idea. Unfortunately, I will not be able to tackle this anytime soon. |
## Motivation and Context - Controlling time is required for several testing use cases - #2087 - #2262 ## Description Introduce `TimeSource` trait, a real implementation, and a test implementation. ## Testing These changes are used in the timestream PR ## Checklist No changelog, these changes have no impact since the code is not yet utilized ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
…hange warning is spurious) (#2728) ## Motivation and Context - #2262 - #2087 - #2707 This adds `TimeSource` in SDK and service configs so we can effectively control time when executing requests with the SDK at the client level. _note:_ the breaking change is in a trait implementation of a struct we're going to delete, not a real breaking change ## Description - Add `SharedTimeSource` and use it in SdkConfig / `aws-config` / <service>::Config - Wire up the signer and other uses of `SystemTime::now()` that I could find - track down broken tests ## Testing - [x] various unit tests that all still pass ## Checklist <!--- If a checkbox below is not applicable, then please DELETE it rather than leaving it unchecked --> - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK 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._
implemented in #2728 |
Motivation and Context
Related to #2087 and awslabs/aws-sdk-rust#679 (reply in thread).
Whenever targeting a platform that the
std::time::SystemTime::now()
orstd::time::Instant::now()
is not supported, it will compile successfully but panic during runtime with the following message:Description
In order to prevent that from happening, we will use the time library with the wasm-bindgen feature flag enabled. The crate
aws-smithy-types
will have that feature enabled by default and we will consume that function to get the current time for the system.Testing
Unit tests already in place
Tested in the browser based on example here: awsdocs/aws-doc-sdk-examples#4325
Checklist
CHANGELOG.next.toml
if I made changes to the smithy-rs codegen or runtime cratesCHANGELOG.next.toml
if I made changes to the AWS SDK, generated SDK code, or SDK runtime cratesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.