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

Add chosen approach explanation to meta document #774

Closed
wants to merge 4 commits into from
Closed

Add chosen approach explanation to meta document #774

wants to merge 4 commits into from

Conversation

shadowhand
Copy link
Contributor

Provide a historical record for why lambda was chosen over double pass.

Refs #771

@shadowhand
Copy link
Contributor Author

@weierophinney do you think I need to add anything else to this regarding the reasoning for lambda?


Further compounding the problem is that there is no way to ensure that the
response body has not been written to, which can lead to incomplete output or
error responses being sent with cache headers attached.
Copy link
Contributor

Choose a reason for hiding this comment

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

Another issue, as detailed in a blog post by Rob Allen, is that it's difficult to get the response body into a clean state if it has been previously written to; rewinding and writing back to it can lead to issues if the new content is of shorter length than the previous. As such, developers currently must depend on a concrete StreamInterface, populate it, and pass it to generate a new response.

@weierophinney
Copy link
Contributor

@shadowhand Provided some notes inline. 😄

@shadowhand
Copy link
Contributor Author

shadowhand commented Jun 20, 2016

@weierophinney updated based on your comments.

This proposal also includes a definition of a middleware stack container that
is an entry point for middleware dispatching. This interface is entirely optional
and is intended to illustrate that client middleware can be used in a server
context but server middleware cannot be used in a client context.
Copy link
Contributor Author

@shadowhand shadowhand Jun 20, 2016

Choose a reason for hiding this comment

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

@weierophinney I've also added this description to the meta document and an additional note the StackInterface. Further comments on the ML.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've replied on the mailing list for this aspect.

Provide a historical record for why lambda was chosen over double pass.

Refs #771
Based on feedback and recommendations for clarity.
Middleware is based on the pipeline pattern. Having more historical
context is useful for understanding.
The stack container is not a critical part of the middleware proposal.
Its primary purpose is to illustrate how to declare type correctly
when operating in server-only, client-only, or mixed stacks.
@shadowhand shadowhand closed this Nov 19, 2016
@shadowhand shadowhand deleted the fix/middleware-chosen-approach branch November 19, 2016 15:52
@shadowhand
Copy link
Contributor Author

Closing this. Will resubmit on a clean branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants