Skip to content

Commit

Permalink
Avoid making FsBuilder aware of http-body version at interface level (
Browse files Browse the repository at this point in the history
#3101)

## Motivation and Context
Follow up on #3088

## Description
This PR reverts `ByteStream::read_with_body_0_4_from`,
`ByteStream::from_path_body_0_4`, and `ByteStream::from_file_body_0_4`
to the old names since from a customers' point of view, `FsBuilder`
should not mention anything about an `http-body` version at the API
level.

`FsBuilder` is currently an opt-in feature (with `rt-tokio` enabled) and
we make it tied to `http-body-0-4-x` by requiring `rt-tokio` including a
dependency on `http-body` version 0.4.x.

## Testing
Relied on the existing tests in CI

## Checklist
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or 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._
  • Loading branch information
ysaito1001 authored Oct 27, 2023
1 parent 78c23de commit 882ebaf
Show file tree
Hide file tree
Showing 12 changed files with 128 additions and 167 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ author = "rcoh"

[[smithy-rs]]
message = "Publicly exposed types from `http-body` and `hyper` crates within `aws-smithy-types` are now feature-gated. See the [upgrade guidance](https://github.com/awslabs/smithy-rs/discussions/3089) for details."
references = ["smithy-rs#3033", "smithy-rs#3088"]
references = ["smithy-rs#3033", "smithy-rs#3088", "smithy-rs#3101"]
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "all" }
author = "ysaito1001"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ mod tests {
let crc32c_checksum = crc32c_checksum.finalize();

let mut request = HttpRequest::new(
ByteStream::read_with_body_0_4_from()
ByteStream::read_from()
.path(&file)
.buffer_size(1024)
.build()
Expand Down
6 changes: 1 addition & 5 deletions aws/sdk/integration-tests/glacier/tests/custom-headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,7 @@ async fn set_correct_headers() {
let _resp = client
.upload_archive()
.vault_name("vault")
.body(
ByteStream::from_path_body_0_4("tests/test-file.txt")
.await
.unwrap(),
)
.body(ByteStream::from_path("tests/test-file.txt").await.unwrap())
.send()
.await;
let req = handler.expect_request();
Expand Down
2 changes: 1 addition & 1 deletion aws/sdk/integration-tests/s3/tests/checksums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ async fn test_checksum_on_streaming_request<'a>(
use std::io::Write;
file.write_all(body).unwrap();

let body = aws_sdk_s3::primitives::ByteStream::read_with_body_0_4_from()
let body = aws_sdk_s3::primitives::ByteStream::read_from()
.path(file.path())
.buffer_size(1024)
.build()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,12 @@ class RequiredCustomizations : ClientCodegenDecorator {
override fun extras(codegenContext: ClientCodegenContext, rustCrate: RustCrate) {
val rc = codegenContext.runtimeConfig

// Add rt-tokio and http-body-0-4-x features for `ByteStream::from_path_0_4`
// Add rt-tokio feature for `ByteStream::from_path`
rustCrate.mergeFeature(
Feature(
"rt-tokio",
true,
listOf("aws-smithy-async/rt-tokio", "aws-smithy-types/rt-tokio", "aws-smithy-types/http-body-0-4-x"),
listOf("aws-smithy-async/rt-tokio", "aws-smithy-types/rt-tokio"),
),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ class ServerRequiredCustomizations : ServerCodegenDecorator {
override fun extras(codegenContext: ServerCodegenContext, rustCrate: RustCrate) {
val rc = codegenContext.runtimeConfig

// Add rt-tokio and http-body-0-4-x features for `ByteStream::from_path_body_0_4`
// Add rt-tokio feature for `ByteStream::from_path`
rustCrate.mergeFeature(
Feature(
"rt-tokio",
true,
listOf("aws-smithy-types/rt-tokio", "aws-smithy-types/http-body-0-4-x"),
listOf("aws-smithy-types/rt-tokio"),
),
)

Expand Down
4 changes: 2 additions & 2 deletions rust-runtime/aws-smithy-http-server-python/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ impl ByteStream {
#[staticmethod]
pub fn from_path_blocking(py: Python, path: String) -> PyResult<Py<PyAny>> {
let byte_stream = Handle::current().block_on(async {
aws_smithy_types::byte_stream::ByteStream::from_path_body_0_4(path)
aws_smithy_types::byte_stream::ByteStream::from_path(path)
.await
.map_err(|e| PyRuntimeError::new_err(e.to_string()))
})?;
Expand All @@ -423,7 +423,7 @@ impl ByteStream {
#[staticmethod]
pub fn from_path(py: Python, path: String) -> PyResult<&PyAny> {
pyo3_asyncio::tokio::future_into_py(py, async move {
let byte_stream = aws_smithy_types::byte_stream::ByteStream::from_path_body_0_4(path)
let byte_stream = aws_smithy_types::byte_stream::ByteStream::from_path(path)
.await
.map_err(|e| PyRuntimeError::new_err(e.to_string()))?;
Ok(Self(Arc::new(Mutex::new(byte_stream))))
Expand Down
2 changes: 1 addition & 1 deletion rust-runtime/aws-smithy-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ repository = "https://github.com/awslabs/smithy-rs"
byte-stream-poll-next = []
http-body-0-4-x = ["dep:http-body-0-4"]
hyper-0-14-x = ["dep:hyper-0-14"]
rt-tokio = ["dep:tokio-util", "dep:tokio", "tokio?/rt", "tokio?/fs", "tokio?/io-util", "tokio-util?/io"]
rt-tokio = ["dep:http-body-0-4", "dep:tokio-util", "dep:tokio", "tokio?/rt", "tokio?/fs", "tokio?/io-util", "tokio-util?/io"]
test-util = []
serde-serialize = []
serde-deserialize = []
Expand Down
7 changes: 3 additions & 4 deletions rust-runtime/aws-smithy-types/src/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,9 @@ impl SdkBody {
/// _Note: This is probably not what you want_
///
/// All bodies constructed from in-memory data (`String`, `Vec<u8>`, `Bytes`, etc.) will be
/// retryable out of the box. If you want to read data from a file, you should turn on a feature
/// `http-body-0-4-x` and use `ByteStream::from_path_body_0_4`.
///
/// This function is only necessary when you need to enable retries for your own streaming container.
/// retryable out of the box. If you want to read data from a file, you should use
/// [`ByteStream::from_path`](crate::byte_stream::ByteStream::from_path). This function
/// is only necessary when you need to enable retries for your own streaming container.
pub fn retryable(f: impl Fn() -> SdkBody + Send + Sync + 'static) -> Self {
let initial = f();
SdkBody {
Expand Down
104 changes: 88 additions & 16 deletions rust-runtime/aws-smithy-types/src/byte_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@
//!
//! ### Create a ByteStream from a file
//!
//! _Note: This is only available with `rt-tokio` and `http-body-0-4-x` enabled._
//! _Note: This is only available with `rt-tokio` enabled._
//!
//! ```no_run
//! # #[cfg(all(feature = "rt-tokio", feature = "http-body-0-4-x"))]
//! # #[cfg(feature = "rt-tokio")]
//! # {
//! use aws_smithy_types::byte_stream::ByteStream;
//! use std::path::Path;
Expand All @@ -90,7 +90,7 @@
//! }
//!
//! async fn bytestream_from_file() -> GetObjectInput {
//! let bytestream = ByteStream::from_path_body_0_4("docs/some-large-file.csv")
//! let bytestream = ByteStream::from_path("docs/some-large-file.csv")
//! .await
//! .expect("valid path");
//! GetObjectInput { body: bytestream }
Expand All @@ -102,7 +102,7 @@
//! or the length of the file, use an `FsBuilder`.
//!
//! ```no_run
//! # #[cfg(all(feature = "rt-tokio", feature = "http-body-0-4-x"))]
//! # #[cfg(feature = "rt-tokio")]
//! # {
//! use aws_smithy_types::byte_stream::{ByteStream, Length};
//! use std::path::Path;
Expand All @@ -111,7 +111,7 @@
//! }
//!
//! async fn bytestream_from_file() -> GetObjectInput {
//! let bytestream = ByteStream::read_with_body_0_4_from().path("docs/some-large-file.csv")
//! let bytestream = ByteStream::read_from().path("docs/some-large-file.csv")
//! .buffer_size(32_784)
//! .length(Length::Exact(123_456))
//! .build()
Expand Down Expand Up @@ -235,26 +235,18 @@ pin_project! {
/// ```
///
/// 2. **From a file**: ByteStreams created from a path can be retried. A new file descriptor will be opened if a retry occurs.
///
/// _Note: The `http-body-0-4-x` feature must be active to call `ByteStream::from_body_0_4`._
///
/// ```no_run
/// #[cfg(all(feature = "tokio-rt", feature = "http-body-0-4-x"))]
/// #[cfg(feature = "tokio-rt")]
/// # {
/// use aws_smithy_types::byte_stream::ByteStream;
/// let stream = ByteStream::from_path_body_0_4("big_file.csv");
/// let stream = ByteStream::from_path("big_file.csv");
/// # }
/// ```
///
/// 3. **From an `SdkBody` directly**: For more advanced / custom use cases, a ByteStream can be created directly
/// from an SdkBody. **When created from an SdkBody, care must be taken to ensure retriability.** An SdkBody is retryable
/// when constructed from in-memory data or when using [`SdkBody::retryable`](crate::body::SdkBody::retryable).
///
/// _Note: The `http-body-0-4-x` feature must be active to construct an `SdkBody` with `from_body_0_4`._
///
/// ```no_run
/// # #[cfg(feature = "http-body-0-4-x")]
/// # {
/// # use hyper_0_14 as hyper;
/// use aws_smithy_types::byte_stream::ByteStream;
/// use aws_smithy_types::body::SdkBody;
Expand All @@ -265,7 +257,6 @@ pin_project! {
/// tx.send_data(Bytes::from_static(b"hello world!"));
/// tx.send_data(Bytes::from_static(b"hello again!"));
/// // NOTE! You must ensure that `tx` is dropped to ensure that EOF is sent
/// # }
/// ```
///
#[derive(Debug)]
Expand Down Expand Up @@ -353,6 +344,87 @@ impl ByteStream {
self.inner.collect().await.map_err(Error::streaming)
}

/// Returns a [`FsBuilder`](crate::byte_stream::FsBuilder), allowing you to build a `ByteStream` with
/// full control over how the file is read (eg. specifying the length of the file or the size of the buffer used to read the file).
/// ```no_run
/// # #[cfg(feature = "rt-tokio")]
/// # {
/// use aws_smithy_types::byte_stream::{ByteStream, Length};
///
/// async fn bytestream_from_file() -> ByteStream {
/// let bytestream = ByteStream::read_from()
/// .path("docs/some-large-file.csv")
/// // Specify the size of the buffer used to read the file (in bytes, default is 4096)
/// .buffer_size(32_784)
/// // Specify the length of the file used (skips an additional call to retrieve the size)
/// .length(Length::Exact(123_456))
/// .build()
/// .await
/// .expect("valid path");
/// bytestream
/// }
/// # }
/// ```
#[cfg(feature = "rt-tokio")]
#[cfg_attr(docsrs, doc(cfg(feature = "rt-tokio")))]
pub fn read_from() -> crate::byte_stream::FsBuilder {
crate::byte_stream::FsBuilder::new()
}

/// Create a ByteStream that streams data from the filesystem
///
/// This function creates a retryable ByteStream for a given `path`. The returned ByteStream
/// will provide a size hint when used as an HTTP body. If the request fails, the read will
/// begin again by reloading the file handle.
///
/// ## Warning
/// The contents of the file MUST not change during retries. The length & checksum of the file
/// will be cached. If the contents of the file change, the operation will almost certainly fail.
///
/// Furthermore, a partial write MAY seek in the file and resume from the previous location.
///
/// Note: If you want more control, such as specifying the size of the buffer used to read the file
/// or the length of the file, use a [`FsBuilder`](crate::byte_stream::FsBuilder) as returned
/// from `ByteStream::read_from`
///
/// # Examples
/// ```no_run
/// use aws_smithy_types::byte_stream::ByteStream;
/// use std::path::Path;
/// async fn make_bytestream() -> ByteStream {
/// ByteStream::from_path("docs/rows.csv").await.expect("file should be readable")
/// }
/// ```
#[cfg(feature = "rt-tokio")]
#[cfg_attr(docsrs, doc(cfg(feature = "rt-tokio")))]
pub async fn from_path(
path: impl AsRef<std::path::Path>,
) -> Result<Self, crate::byte_stream::error::Error> {
crate::byte_stream::FsBuilder::new()
.path(path)
.build()
.await
}

/// Create a ByteStream from a file
///
/// NOTE: This will NOT result in a retryable ByteStream. For a ByteStream that can be retried in the case of
/// upstream failures, use [`ByteStream::from_path`](ByteStream::from_path)
#[deprecated(
since = "0.40.0",
note = "Prefer the more extensible ByteStream::read_from() API"
)]
#[cfg(feature = "rt-tokio")]
#[cfg_attr(docsrs, doc(cfg(feature = "rt-tokio")))]
pub async fn from_file(
file: tokio::fs::File,
) -> Result<Self, crate::byte_stream::error::Error> {
crate::byte_stream::FsBuilder::new()
.file(file)
.build()
.await
}

#[cfg(feature = "rt-tokio")]
/// Convert this `ByteStream` into a struct that implements [`AsyncRead`](tokio::io::AsyncRead).
///
Expand Down
Loading

0 comments on commit 882ebaf

Please sign in to comment.