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.

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

Remove futures_core::stream::Stream from aws-smithy-async #2978

Merged
merged 14 commits into from
Sep 27, 2023

Conversation

ysaito1001
Copy link
Contributor

@ysaito1001 ysaito1001 commented Sep 9, 2023

Motivation and Context

Removes futures_core::stream::Stream from the aws-smithy-async crate.

Description

This PR is part of our ongoing effort, #2413. We remove the futures_core::stream::Stream trait from the public API in the aws-smithy-async crate. While doing so, we compensate

  • FnStream by providing the explicit .next and .collect methods to let the previously working code continue working.
  • TryFlatMap by making it return a new-type wrapper PaginationStream to hide FnStream from those who use paginators.

With this change, the latest canary no longer uses tokio_stream::StreamExt, since the paginator does not work in terms of the Stream trait. Furthermore, aws-sdk-rust has been more than 3 releases since release-2023-01-26, so the module release-2023-01-26 has been removed from canary-lambda.

Testing

No new tests added, but made sure the existing tests keep working with the change.

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
Copy link

github-actions bot commented Sep 9, 2023

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link
Collaborator

@rcoh rcoh left a comment

Choose a reason for hiding this comment

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

nice! like the direction here.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@ysaito1001 ysaito1001 marked this pull request as ready for review September 11, 2023 17:27
@ysaito1001 ysaito1001 requested review from a team as code owners September 11, 2023 17:27
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@ysaito1001 ysaito1001 requested a review from rcoh September 20, 2023 19:58
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@ysaito1001 ysaito1001 added the breaking-change This will require a breaking change label Sep 20, 2023
Copy link
Collaborator

@rcoh rcoh left a comment

Choose a reason for hiding this comment

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

Looking good! Just a couple of small tweaks. Nice work on this.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@ysaito1001 ysaito1001 added this pull request to the merge queue Sep 27, 2023
Merged via the queue into main with commit 75fc85f Sep 27, 2023
40 of 41 checks passed
@ysaito1001 ysaito1001 deleted the ysaito/remove-futures-stream-from-smithy-async branch September 27, 2023 21:39
github-merge-queue bot pushed a commit that referenced this pull request Sep 29, 2023
## Motivation and Context
Fixes [canary
failures](https://github.com/awslabs/aws-sdk-rust/actions/runs/6345490441/job/17237658244)
by updating the-last-release-before-breaking-change module in
`canary-lambda`.

## Description
[A previous PR](#2978) removing
the `Steram` trait from public API had a breaking change, requiring
canary to update its code. The latest release at the time while the PR
was worked on was `release-2023-08-23` so we created a new module with
that date and moved the previously working canary's code prior to the
breaking change to it. The `latest` module then contained canary's code
after the breaking change.

However, before that PR was merged to main, we made several releases in
`aws-sdk-rust`, making `release-2023-08-23` no longer the latest release
before the breaking change; a new one is `release_2023_09_25`. Hence
this PR.

## Testing
Manually executed
```
➜ canary-runner git:(ysaito/fix-canary-module-dates) cargo run -- build-bundle --canary-path ../canary-lambda --sdk-release-tag release-2023-09-25 --musl --manifest-only
➜ canary-runner git:(ysaito/fix-canary-module-dates) cd ../canary-lambda
➜ canary-lambda git:(ysaito/fix-canary-module-dates) cargo check
```
(also tested for `--sdk-release-tag` with `release-2023-09-22` and
`release-2023-09-20`)

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This will require a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants