Skip to content

Replace Model trait with Format trait (applied to Response instead) #2559

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
6 changes: 5 additions & 1 deletion sdk/core/azure_core/src/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,9 @@ pub use response::Response;

pub use typespec_client_core::http::response;
pub use typespec_client_core::http::{
new_http_client, AppendToUrlQuery, Context, HttpClient, Method, StatusCode, Url,
new_http_client, AppendToUrlQuery, Context, Format, HttpClient, JsonFormat, Method, StatusCode,
Url,
};

#[cfg(feature = "xml")]
pub use typespec_client_core::http::XmlFormat;
13 changes: 8 additions & 5 deletions sdk/core/azure_core/src/http/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::{
ops::Deref,
sync::Arc,
};
use typespec_client_core::http::{self, policies::Policy};
use typespec_client_core::http::{self, policies::Policy, Format, JsonFormat};

/// Execution pipeline.
///
Expand All @@ -34,14 +34,17 @@ use typespec_client_core::http::{self, policies::Policy};
/// cannot be enforced by code). All policies except Transport policy can assume there is another following policy (so
/// `self.pipeline[0]` is always valid).
#[derive(Debug, Clone)]
pub struct Pipeline(http::Pipeline);
pub struct Pipeline<F: Format = JsonFormat>(http::Pipeline<F>);

impl Pipeline {
impl<F: Format> Pipeline<F> {
Copy link
Member

Choose a reason for hiding this comment

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

Interesting. Was this a necessary viral change? Though, it's congruent with our discussion offline about requiring separate clients for JSON, XML, or whatever future format we might support. @JeffreyRichter, thoughts? I can catch you up offline. Basically, the whole point of this PR is to simplify callers because the client knows what the format is, so there's no reason to pass that responsibility off to callers as we have been.

I guess what I wasn't expecting is that we couldn't do this only in client methods' impl, but is that even a practical concern? Would we have any scenarios where - using the same client instance - we want to send e.g., both JSON and XML? Seems incredibly unlikely, though not impossible. (Unbranding might be more of a concern.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you referring specifically to the trait bound on F? The type parameter is certainly viral though, as only the client itself knows what format to use for a Pipeline.

Copy link
Member

Choose a reason for hiding this comment

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

Let's discuss offline

Copy link
Member

Choose a reason for hiding this comment

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

I put something on our calendars, but we can chat about it as well if people would rather.

/// Creates a new pipeline given the client library crate name and version,
/// alone with user-specified and client library-specified policies.
///
/// Crates can simply pass `option_env!("CARGO_PKG_NAME")` and `option_env!("CARGO_PKG_VERSION")` for the
/// `crate_name` and `crate_version` arguments respectively.
///
/// In addition, this constructor allows for specifying the response format (e.g. JSON, XML) to be used
/// when deserializing the response body.
pub fn new(
crate_name: Option<&'static str>,
crate_version: Option<&'static str>,
Expand Down Expand Up @@ -75,8 +78,8 @@ impl Pipeline {
}

// TODO: Should we instead use the newtype pattern?
impl Deref for Pipeline {
type Target = http::Pipeline;
impl<F: Format> Deref for Pipeline<F> {
type Target = http::Pipeline<F>;
fn deref(&self) -> &Self::Target {
&self.0
}
Expand Down
15 changes: 8 additions & 7 deletions sdk/storage/azure_storage_blob/src/clients/blob_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use azure_core::{
Bytes, Result,
};
use std::sync::Arc;
use typespec_client_core::http::XmlFormat;

/// A client to interact with a specific Azure storage blob, although that blob may not yet exist.
pub struct BlobClient {
Expand Down Expand Up @@ -114,7 +115,7 @@ impl BlobClient {
pub async fn get_properties(
&self,
options: Option<BlobClientGetPropertiesOptions<'_>>,
) -> Result<Response<BlobClientGetPropertiesResult>> {
) -> Result<Response<BlobClientGetPropertiesResult, XmlFormat>> {
self.client.get_properties(options).await
}

Expand All @@ -126,7 +127,7 @@ impl BlobClient {
pub async fn set_properties(
&self,
options: Option<BlobClientSetPropertiesOptions<'_>>,
) -> Result<Response<()>> {
) -> Result<Response<(), XmlFormat>> {
self.client.set_properties(options).await
}

Expand All @@ -136,7 +137,7 @@ impl BlobClient {
pub async fn download(
&self,
options: Option<BlobClientDownloadOptions<'_>>,
) -> Result<Response<BlobClientDownloadResult>> {
) -> Result<Response<BlobClientDownloadResult, XmlFormat>> {
self.client.download(options).await
}

Expand All @@ -155,7 +156,7 @@ impl BlobClient {
overwrite: bool,
content_length: u64,
options: Option<BlockBlobClientUploadOptions<'_>>,
) -> Result<Response<BlockBlobClientUploadResult>> {
) -> Result<Response<BlockBlobClientUploadResult, XmlFormat>> {
let mut options = options.unwrap_or_default();

if !overwrite {
Expand All @@ -179,7 +180,7 @@ impl BlobClient {
pub async fn set_metadata(
&self,
options: Option<BlobClientSetMetadataOptions<'_>>,
) -> Result<Response<()>> {
) -> Result<Response<(), XmlFormat>> {
self.client.set_metadata(options).await
}

Expand All @@ -191,7 +192,7 @@ impl BlobClient {
pub async fn delete(
&self,
options: Option<BlobClientDeleteOptions<'_>>,
) -> Result<Response<()>> {
) -> Result<Response<(), XmlFormat>> {
self.client.delete(options).await
}

Expand All @@ -206,7 +207,7 @@ impl BlobClient {
&self,
tier: AccessTier,
options: Option<BlobClientSetTierOptions<'_>>,
) -> Result<Response<()>> {
) -> Result<Response<(), XmlFormat>> {
self.client.set_tier(tier, options).await
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use azure_core::{
Result,
};
use std::sync::Arc;
use typespec_client_core::http::XmlFormat;

/// A client to interact with a specified Azure storage container.
pub struct BlobContainerClient {
Expand Down Expand Up @@ -99,7 +100,7 @@ impl BlobContainerClient {
pub async fn create_container(
&self,
options: Option<BlobContainerClientCreateOptions<'_>>,
) -> Result<Response<()>> {
) -> Result<Response<(), XmlFormat>> {
self.client.create(options).await
}

Expand All @@ -113,7 +114,7 @@ impl BlobContainerClient {
pub async fn set_metadata(
&self,
options: Option<BlobContainerClientSetMetadataOptions<'_>>,
) -> Result<Response<()>> {
) -> Result<Response<(), XmlFormat>> {
self.client.set_metadata(options).await
}

Expand All @@ -125,7 +126,7 @@ impl BlobContainerClient {
pub async fn delete_container(
&self,
options: Option<BlobContainerClientDeleteOptions<'_>>,
) -> Result<Response<()>> {
) -> Result<Response<(), XmlFormat>> {
self.client.delete(options).await
}

Expand All @@ -138,7 +139,7 @@ impl BlobContainerClient {
pub async fn get_properties(
&self,
options: Option<BlobContainerClientGetPropertiesOptions<'_>>,
) -> Result<Response<BlobContainerClientGetPropertiesResult>> {
) -> Result<Response<BlobContainerClientGetPropertiesResult, XmlFormat>> {
self.client.get_properties(options).await
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl BlockBlobClient {
&self,
blocks: RequestContent<BlockLookupList>,
options: Option<BlockBlobClientCommitBlockListOptions<'_>>,
) -> Result<Response<BlockBlobClientCommitBlockListResult>> {
) -> Result<Response<BlockBlobClientCommitBlockListResult, XmlFormat>> {
self.client.commit_block_list(blocks, options).await
}

Expand All @@ -124,7 +124,7 @@ impl BlockBlobClient {
content_length: u64,
body: RequestContent<Bytes>,
options: Option<BlockBlobClientStageBlockOptions<'_>>,
) -> Result<Response<BlockBlobClientStageBlockResult>> {
) -> Result<Response<BlockBlobClientStageBlockResult, XmlFormat>> {
self.client
.stage_block(block_id, content_length, body, options)
.await
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading