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

fix(lib-dynamodb): pass lib Command middleware to client Commands #3756

Closed
wants to merge 1 commit into from

Conversation

kuhe
Copy link
Contributor

@kuhe kuhe commented Jun 28, 2022

Issue

#3095

Description

  • adds .clientMiddlewareStack to lib-dynamodb commands. This stack will be applied to the underlying client middleware as it is initialized.
  • .middlewareStack from extending smithy Command is not usable because it implies IO types that will not actually appear in the middleware at runtime (the marshalled types).

Testing

  • add unit test

@kuhe kuhe added the pr/needs-review This PR needs a review from a Member. label Jun 28, 2022
@kuhe kuhe requested a review from a team as a code owner June 28, 2022 16:23
@kuhe kuhe self-assigned this Jun 28, 2022
@kuhe
Copy link
Contributor Author

kuhe commented Jun 28, 2022

supercedes #3748

@kuhe kuhe changed the title 3095 lib dynamodb fix(lib-dynamodb): pass lib Command middleware to client Commands Jun 28, 2022
@kuhe kuhe marked this pull request as draft June 30, 2022 19:00
@kuhe kuhe force-pushed the 3095-lib-dynamodb branch 2 times, most recently from 1ade74a to 02c1ab2 Compare July 1, 2022 15:24
const { marshallOptions, unmarshallOptions } = configuration.translateConfig || {};
const command = new __QueryCommand(marshallInput(this.input, this.inputKeyNodes, marshallOptions));
const handler = command.resolveMiddleware(clientStack, configuration, options);
const command = new __QueryCommand(this.input);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

add ts-ignore for future possibility of a command with non-optional fields

@kuhe
Copy link
Contributor Author

kuhe commented Jul 7, 2022

discussed with team in a review document, here's what we want to do with the PR:

  • ensure no potential for type errors in build. The current polymorphism may only be getting through because all the I/O types have optional fields, or due to some level of indirection obscuring the type shenanigans
  • create new PR to supercede this, leaving it as a record
  • initialize inner command in ctor, but not public
  • make LibCommand.mwStack simply passthrough ref to the inner command mwStack

@github-actions
Copy link

github-actions bot commented Aug 3, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr/needs-review This PR needs a review from a Member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant