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

Support passing end_of_stream to header callbacks #21

Closed
wants to merge 1 commit into from
Closed

Support passing end_of_stream to header callbacks #21

wants to merge 1 commit into from

Conversation

gbrail
Copy link
Contributor

@gbrail gbrail commented May 21, 2020

Do this in a backwardly-compatible way by creating two new ABI
functions:

on_request_headers_ext
on_response_headers_ext

Each one takes an additional Word parameter at the end which
carries the value of the "end_of_stream" flag from Envoy.
The proxy_wasm_cpp_host code will first check for the existence
of the "ext" version of a function and fall back to the original
version if it is not found.

If you like this approach, then I'll submit the matching PR to fix this in the "host" repo.

If you would prefer to simply add the additional flag to the existing function signatures,
then I'm happy to do that as well. It would be an incompatible change to the ABI, although I think that one of those was already made when the function to get the configuration data was removed.

Signed-off-by: Gregory Brail gregbrail@google.com

Do this in a backwardly-compatible way by creating two new ABI
functions:

on_request_headers_ext
on_response_headers_ext

Each one takes an additional Word parameter at the end which
carries the value of the "end_of_stream" flag from Envoy.
The proxy_wasm_cpp_host code will first check for the existence
of the "ext" version of a function and fall back to the original
version if it is not found.

Signed-off-by: Gregory Brail <gregbrail@google.com>
Copy link
Contributor

@jplevyak jplevyak left a comment

Choose a reason for hiding this comment

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

We do not yet have to be compatible. Let's just make a set of related changes to the SDK and host support code.

@gbrail
Copy link
Contributor Author

gbrail commented May 28, 2020 via email

@gbrail
Copy link
Contributor Author

gbrail commented May 28, 2020

I'll drop this in favor of a new one.

@gbrail gbrail closed this May 28, 2020
@gbrail gbrail deleted the add-headers-end-flag branch May 28, 2020 21:46
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

2 participants