Add support for TimeStreamWrite and TimeStreamQuery #2707

rcoh · 2023-05-17T18:03:52Z

TODO:

docs
integration test (canary even?)
customize README for timestream

Motivation and Context

Description

This adds support for TSW and TSQ by adding endpoint discovery as a customization. This is made much simpler by the fact that endpoint discovery for these services has no parameters which means that there is no complexity from caching the returned endpoint.

Customers call .enable_endpoint_discovery() on the client to create a version of the client with endpoint discovery enabled. This returns a new client and a Reloader from which customers must spawn the reload task if they want endpoint discovery to rerun.

Testing

Checklist

I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates
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.

github-actions · 2023-05-17T18:32:04Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

jdisanti

This looks good, but seems like it is missing tests.

jdisanti · 2023-05-17T19:49:27Z

aws/rust-runtime/aws-credential-types/src/cache/expiring_cache.rs

+    ///
+    /// # Panics
+    /// This function panics if it is called from an asynchronous context
+    pub fn try_blocking_get(&self) -> Option<T> {


Doesn't look like this is used?

yeah deleted, an artifact from a previous impl

...n/src/main/kotlin/software/amazon/smithy/rustsdk/customize/timestream/TimestreamDecorator.kt

jdisanti · 2023-05-17T19:57:26Z

...oftware/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt

@@ -204,6 +204,7 @@ class ServiceConfigGenerator(private val customizations: List<ConfigCustomizatio
        customizations.forEach {
            it.section(ServiceConfig.ConfigStructAdditionalDocs)(writer)
        }
+        Attribute(Attribute.derive(RuntimeType.Clone)).render(writer)


Would be awesome if this could also derive Debug instead of the Debug impl that just outputs "Config"

…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  - [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._

This adds support for TSW and TSQ by adding endpoint discovery as a customization. This is made much simpler by the fact that endpoint discovery for these services **has no parameters** which means that there is no complexity from caching the returned endpoint. Customers call `.enable_endpoint_discovery()` on the client to create a version of the client with endpoint discovery enabled. This returns a new client and a Reloader from which customers must spawn the reload task if they want endpoint discovery to rerun.

github-actions · 2023-05-31T14:47:59Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2023-05-31T15:25:15Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

Velfi · 2023-05-31T15:12:56Z

aws/rust-runtime/aws-inlineable/src/endpoint_discovery.rs

+                _ => {}
+            }
+            self.reload_increment(self.time.now()).await;
+            self.sleep.sleep(Duration::from_secs(60)).await;


How was the 1min increment decided? A comment would be appreciated.

Velfi · 2023-05-31T15:27:36Z

aws/rust-runtime/aws-inlineable/src/endpoint_discovery.rs

+        tracing::debug!(expiry = ?self.expiry, now = ?now, delta = ?self.expiry.duration_since(now), "checking expiry status of endpoint");
+        match self.expiry.duration_since(now) {
+            Err(_) => true,
+            Ok(t) => t < Duration::from_secs(120),


I'm guessing this value is meant to always be 2x the 1min duration, perhaps linking them together with consts would be a good idea. Of course, if we're never going to touch this stuff again then it's not important.

Velfi · 2023-05-31T15:29:30Z

aws/rust-runtime/aws-inlineable/src/endpoint_discovery.rs

+    // if we didn't successfully get an endpoint, bail out so the client knows
+    // configuration failed to work


Suggested change

// if we didn't successfully get an endpoint, bail out so the client knows

// configuration failed to work

// if we failed to get an endpoint from the cache, bail out so that the client knows

// that configuration failed

Velfi · 2023-05-31T15:30:07Z

aws/rust-runtime/aws-inlineable/src/endpoint_discovery.rs

+            .lock()
+            .unwrap()
+            .as_ref()
+            .map(|e| e.endpoint.clone())


Do we need a map? Can't we just call clone directly?

Suggested change

.map(|e| e.endpoint.clone())

.clone()

this is plucking out the endpoint in the map (not just cloning)

Velfi · 2023-05-31T15:34:56Z

...n/src/main/kotlin/software/amazon/smithy/rustsdk/customize/timestream/TimestreamDecorator.kt

+ * 1. Adds the `endpoint_discovery` inlineable
+ * 2. Adds a `enable_endpoint_discovery` method on client that returns a wrapped client with endpoint discovery enabled
+ */
+class TimestreamDecorator : ClientCodegenDecorator {


Did you actually use the crate docs decorator anywhere? I would have assumed there'd be something in here about how timestream support is experimental.

I generated docs that add enable_endpoint_discovery—I figure our standard boilerplate is probably good for starters

rcoh requested a review from a team as a code owner May 17, 2023 18:03

rcoh force-pushed the timestream-v1 branch from 0017ff1 to 0eef47b Compare May 17, 2023 18:04

jdisanti reviewed May 17, 2023

View reviewed changes

rcoh requested a review from a team as a code owner May 23, 2023 16:50

rcoh force-pushed the timestream-v1 branch from ce3b5d7 to cd9a800 Compare May 24, 2023 19:17

rcoh mentioned this pull request May 25, 2023

Integrate TimeSource into the orchestrator and middleware (breaking change warning is spurious) #2728

Merged

3 tasks

rcoh force-pushed the timestream-v1 branch 2 times, most recently from 760d379 to 47892e6 Compare May 31, 2023 12:48

Add tests of timestream + docs

fdb0076

rcoh force-pushed the timestream-v1 branch from 47892e6 to fdb0076 Compare May 31, 2023 13:15

rcoh added 2 commits May 31, 2023 09:30

Update changelog and fix docs to include .await

347be17

Filter addition of the awsConfig dependency to avoid circular dep

77018c5

rcoh force-pushed the timestream-v1 branch from dfe39c2 to f614ff8 Compare May 31, 2023 14:18

fix lint breakage

be66955

rcoh force-pushed the timestream-v1 branch from f614ff8 to be66955 Compare May 31, 2023 14:49

Velfi approved these changes May 31, 2023

View reviewed changes

Velfi reviewed May 31, 2023

View reviewed changes

rcoh added this pull request to the merge queue May 31, 2023

Merged via the queue into main with commit 8c045d2 May 31, 2023

rcoh deleted the timestream-v1 branch May 31, 2023 18:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for TimeStreamWrite and TimeStreamQuery #2707

Add support for TimeStreamWrite and TimeStreamQuery #2707

rcoh commented May 17, 2023 •

edited

Loading

github-actions bot commented May 17, 2023

jdisanti left a comment

jdisanti May 17, 2023

rcoh May 17, 2023

jdisanti May 17, 2023

github-actions bot commented May 31, 2023

github-actions bot commented May 31, 2023

Velfi May 31, 2023

Velfi May 31, 2023

Velfi May 31, 2023

Velfi May 31, 2023

rcoh May 31, 2023

Velfi May 31, 2023

rcoh May 31, 2023

		// if we didn't successfully get an endpoint, bail out so the client knows
		// configuration failed to work

Add support for TimeStreamWrite and TimeStreamQuery #2707

Add support for TimeStreamWrite and TimeStreamQuery #2707

Conversation

rcoh commented May 17, 2023 • edited Loading

Motivation and Context

Description

Testing

Checklist

github-actions bot commented May 17, 2023

jdisanti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 31, 2023

github-actions bot commented May 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcoh commented May 17, 2023 •

edited

Loading