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

widen blob payload input types, add blob payload output converter #777

Merged
merged 2 commits into from
Jun 20, 2023

Conversation

kuhe
Copy link
Contributor

@kuhe kuhe commented Jun 13, 2023

aws/aws-sdk-js-v3#4786
aws/aws-sdk-js-v3#4265

Blob types enhancements

  • on the client input side, operation input members with type blob and trait HttpPayload have been widened from Uint8Array to BlobTypes, which contains common and runtime specific types.
export type BlobTypes = string | ArrayBuffer | ArrayBufferView | Uint8Array | RuntimeBlobTypes;

// example Node.js runtime additions
export type RuntimeBlobTypes = Readable | Buffer;
// browser additions
export type RuntimeBlobTypes = Blob | ReadableStream;

This applies in general, but the user story is difficulty/inconvenience around non-streaming blob i/o, e.g. in Lambda invoke. Non-streaming blobs were treated uniformly as Uint8Array in input and output, but this does not cover the realistic use cases of payload blobs.

const lambda = new Lambda({
  region: "us-west-2",
});

const invoke = lambda.invoke({
  FunctionName: "echo",
  Payload: JSON.stringify({
    hello: "world",
  }), // without input widening, this would be a type error since string is not Uint8Array.
});

const payload = (await invoke).Payload;

console.log(JSON.parse(payload?.transformToString() ?? "{}")); // adapter adds the string conversion method, preserves the type extending Uint8Array.
  • JSv3 will write an e2e integration test suite for this.

@kuhe kuhe force-pushed the feat/blob branch 5 times, most recently from c50b9cb to 0178742 Compare June 15, 2023 18:30
@kuhe kuhe marked this pull request as ready for review June 15, 2023 19:44
@kuhe kuhe requested review from a team as code owners June 15, 2023 19:44
@kuhe kuhe force-pushed the feat/blob branch 3 times, most recently from 14bb492 to a909bc5 Compare June 16, 2023 21:10
@@ -248,25 +248,14 @@ static void generateMetadataDeserializer(GenerationContext context, SymbolRefere
}

/**
* Writes a response body stream collector. This function converts the low-level response body stream to
* Imports a response body stream collector. This function converts the low-level response body stream to
* Uint8Array binary data.
*
* @param context The generation context.
*/
static void generateCollectBody(GenerationContext context) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this whole method if it just imports something? Or maybe change the name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I rolled it into the generateCollectBodyString method, which is called in all places that generateCollectBody was called.

@kuhe kuhe force-pushed the feat/blob branch 2 times, most recently from e1e48c4 to ca724d4 Compare June 20, 2023 21:03
@kuhe kuhe merged commit 6d624aa into smithy-lang:main Jun 20, 2023
7 checks passed
@kuhe kuhe deleted the feat/blob branch June 20, 2023 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants