Upgrade AWS crates and localstack #269

oliverdaff · 2024-01-15T12:22:29Z

📢 For the Reviewer

In this PR, we're updating various AWS SDK dependencies for the cobalt-aws project to align with the latest versions and introducing support for LocalStack 3.0.2. Please provide feedback following the conventional comments guidebook to streamline the review process.

🚦 Depends on

No other PRs.

🛠️ What

This PR includes the following key changes:

Dependency Updates:
- aws-sdk-athena: Upgraded from 0.29 to 1.10.0.
- aws-config: Updated to 1.1.2 with the behavior-version-latest feature.
- aws-sdk-s3: Upgraded from 0.29 to 1.12.0.
- aws-sdk-sqs: Upgraded from 0.29 to 1.10.0.
- aws-types: Upgraded from 0.56 to 1.1.2.
- aws-smithy-async: Added 1.1.2.
- aws-smithy-runtime-api: Added 1.1.2.
- aws-smithy-http: Removed.
New Internal Structs:
- FuturesStreamCompatByteStream: Implemented to create a Futures stream from a ByteStream to maintain API to return a futures::AsyncBufRead .
- FuturesPaginationStream: Designed to create a stream from a PaginationStream, enhancing asynchronous operations. AWS does not provide runtime Stream implementations for the PaginationStream, this provides a futures implementation.
LocalStack Upgrade: Upgraded to LocalStack 3.0.2, involving modifications in initializing LocalStack and ensuring its proper initialization.

❓ Why

The primary goals for this PR are:

Keeping Dependencies Updated: Ensuring our project uses the latest versions of dependencies for enhanced security, performance, and access to new features.
Maintain Asynchronous Support: By introducing FuturesStreamCompatByteStream and FuturesPaginationStream, we aim to maintain our asynchronous data operations.
Compatibility with LocalStack 3.0.2: The upgrade is crucial for maintaining compatibility with the latest local AWS cloud stack, which is essential for testing and development purposes.

😟 Concerns

Integration Testing: Given the significant dependency updates and new features, extensive integration testing is necessary to ensure compatibility and stability.
LocalStack Changes: The LocalStack upgrade might introduce unforeseen challenges during local development and testing.

📝 Notes

During the process of passing the integration test, it was observed that the cargo-deny version 0.14.3 in the rust base image led to failures in executing cargo deny check. To resolve this, a docker file was introduced to revert to version 0.14.1, the earliest version that operates correctly. This temporary local modification was chosen to expedite the review and merging of this ticket. The plan is to propose a change for downgrading cargo-deny in the primary docker repository. The update will be applied to this ticket or a new one will be created in this repository to eliminate the rust-dev.dockerfile after the primary repository releases a new version. It's important to note that the issue with cargo-deny was exclusively found when running it inside docker.

Upgraded Cargo Updated localstack Fixed use of localstack in integration tests.

timleslie

I'm still working my way through this, but need to context switch right now. I'll leave the comments I have so far.

Overall looks great, I don't see any major issue, but I still need to look more closely at the new functions that have been added.

Something for us to consider. Given this is a significant breaking change that takes us to 1.0 of the AWS SDK, when we release this should we call it 1.0.0? Related to that, before we land this can we update the CHANGELOG.md with a summary of any changes that users of cobalt-aws will need to be aware of?

docker-compose.yml

src/s3/mod.rs

timleslie · 2024-01-16T21:32:30Z

Cargo.toml

 serde = "1.0"
 serde_json = "1.0"
 tokio = { version = "1.35", features=["macros"] }
 tracing = "0.1"
 tracing-subscriber = { version = "0.3", features=["json", "env-filter"] }
 url = { version = "2.5", features = ["serde"] }
+bytes = "1.5.0"


nitpick: To help with readability these dependencies are maintained in alphabetical order. Could we move this new dependency up?

(This is another case where a linter would be an ideal solution to keep things tidy (in the eye of the beholder)).

timleslie · 2024-01-16T21:34:49Z

src/lambda.rs

@@ -301,7 +301,7 @@ where
    // for future work). To work around this, we capture any errors during this phase
    // and pass them into the handler function itself, so that it can raise the error
    // when the function is invoked.
-    let init_result = (|| async {
+    let init_result = async {


comment: This is much nicer! Do you know if this syntax has always worked? I seem to recall wrestling with this code when I first wrote it to get it to behave how I wanted. I wonder if I just missed this simplification, or if something in the language has changed 🤔

As far as I know the syntax let example_future = async { ... }; has been valid since Rust 1.39.0, released in November 2019. If you were working with Rust before this version, it's possible you might have missed this simplification at the time. Earlier versions required using a feature flag for async/await, so the language's capabilities have evolved significantly since then.

comozo

Thanks for taking the challenge of upgrading, it is not an easy feat. From my standpoint it looks great. 👍🏿

comozo · 2024-01-16T23:00:40Z

Makefile

@@ -12,7 +12,7 @@ test:
 # Sleep for 20s in between to let CI complete execution


nitpick: change 20s to 1s to match the change below

comozo · 2024-01-16T23:03:02Z

docker-compose.yml

@@ -49,10 +54,11 @@ services:
      - LOCALSTACK_API_KEY=${LOCALSTACK_API_KEY-}  # only required for Pro
      - DOCKER_HOST=unix:///var/run/docker.sock
      - AWS_DEFAULT_REGION=ap-southeast-2
+      - DEFAULT_REGION=ap-southeast-2


question: What is using this env var ?

This should be removed, it was something I tried while trying to get the tests to pass.

comozo · 2024-01-16T23:04:28Z

rust-dev.dockerfile

@@ -0,0 +1,25 @@
+FROM ghcr.io/harrison-ai/rust:1.75-1.0
+
+# cargo deny version 0.14.3 in the rust base layer fails on cargo deny check with:


praise: Thanks for the comment. Our future self will be happy to find 😄

comozo · 2024-01-16T23:09:17Z

src/s3/mod.rs

@@ -26,6 +35,70 @@ pub use async_multipart_put_object::AsyncMultipartUpload;
 pub use async_put_object::AsyncPutObject;
 pub use s3_object::S3Object;

+/// `FuturesStreamCompatByteStream` is a compatibility layer struct designed to wrap


praise: Awesome comment. I've learned something. Thanks

Co-authored-by: Tim Leslie <[email protected]>

Upgrade dependencies

46bae53

Upgraded Cargo Updated localstack Fixed use of localstack in integration tests.

oliverdaff force-pushed the feat/aws-sdk-1.0 branch from 7f7c860 to 46bae53 Compare January 16, 2024 06:25

oliverdaff added 2 commits January 16, 2024 17:21

Updated http lib

f6fff5a

Use a working version of cargo-deny

5578880

oliverdaff marked this pull request as ready for review January 16, 2024 10:59

oliverdaff requested a review from a team as a code owner January 16, 2024 10:59

timleslie reviewed Jan 16, 2024

View reviewed changes

comozo approved these changes Jan 16, 2024

View reviewed changes

oliverdaff and others added 4 commits January 17, 2024 11:44

Remove unused env var

cb91cc5

Make alphabetical

f8adec5

Remove sleep

cea0e32

Apply suggestions from code review

41f4795

Co-authored-by: Tim Leslie <[email protected]>

oliverdaff added this pull request to the merge queue Jan 17, 2024

Merged via the queue into main with commit 0d4016e Jan 17, 2024
3 checks passed

oliverdaff deleted the feat/aws-sdk-1.0 branch January 17, 2024 02:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade AWS crates and localstack #269

Upgrade AWS crates and localstack #269

oliverdaff commented Jan 15, 2024 •

edited

Loading

timleslie left a comment

timleslie Jan 16, 2024

timleslie Jan 16, 2024

oliverdaff Jan 17, 2024

comozo left a comment

comozo Jan 16, 2024

comozo Jan 16, 2024

oliverdaff Jan 17, 2024

comozo Jan 16, 2024

comozo Jan 16, 2024

		@@ -12,7 +12,7 @@ test:
		# Sleep for 20s in between to let CI complete execution

		@@ -0,0 +1,25 @@
		FROM ghcr.io/harrison-ai/rust:1.75-1.0

		# cargo deny version 0.14.3 in the rust base layer fails on cargo deny check with:

Upgrade AWS crates and localstack #269

Upgrade AWS crates and localstack #269

Conversation

oliverdaff commented Jan 15, 2024 • edited Loading

📢 For the Reviewer

🚦 Depends on

🛠️ What

❓ Why

😟 Concerns

📝 Notes

timleslie 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

comozo 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliverdaff commented Jan 15, 2024 •

edited

Loading