Skip to content

Add SSE style message framing for streamed responses #39325

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 2 commits into
base: main
Choose a base branch
from

Conversation

numanelahi
Copy link
Contributor

Commit Message: Add SSE style message framing for streamed responses
Additional Description: This PR adds Server Side Events(SSE) style message framing for the streamed responses transcoded by the gRPC JSON transcoder. Before this the responses were streamed either as JSON array or as NDJSON.
Risk Level: low
Testing: Added unit tests for the feature
Docs Changes: Add details about the new config param for this feature.
Release Notes: Added to the change log.
Platform Specific Features: n/a

@numanelahi numanelahi requested a review from nezdolik as a code owner May 5, 2025 07:29
@repokitteh-read-only repokitteh-read-only bot added api deps Approval required for changes to Envoy's external dependencies labels May 5, 2025
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @abeyad
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).
CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @moderation

🐱

Caused by: #39325 was opened by numanelahi.

see: more, trace.

@numanelahi
Copy link
Contributor Author

The Envoy/Prechecks are failing in ./ci/do_ci.sh deps with the following error: aio.core.tasks.exceptions.ConcurrentExecutionError: Download failed https://nvd.nist.gov/feeds/json/cve/1.1/nvdcve-1.1-2022.json.gz: Not Found. This error seems unrelated to the my changes. When I try to download the nvdcve-1.1-2022.json.gz with the same link with out the trailing : it works. The error is probably because of the colon : at the end of the url.

@phlax
Copy link
Member

phlax commented May 5, 2025

its a very annoying flake - i take full responsibility - but the tool is badly coded and badly cached ... such that if it caches failure you cant easily recache - still worth a try ...

/retest

abeyad
abeyad previously approved these changes May 5, 2025
Copy link
Contributor

@abeyad abeyad left a comment

Choose a reason for hiding this comment

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

/lgtm api

thanks @numanelahi !

@repokitteh-read-only repokitteh-read-only bot removed the api label May 5, 2025
@abeyad
Copy link
Contributor

abeyad commented May 5, 2025

/assign @nezdolik

as grpc_json_transcoder code owner

Copy link
Contributor

@moderation moderation left a comment

Choose a reason for hiding this comment

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

Updated release date required

nezdolik
nezdolik previously approved these changes May 6, 2025
@numanelahi
Copy link
Contributor Author

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps Approval required for changes to Envoy's external dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants