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

Refactor descrption of events #2

Open
paulmillar opened this issue Oct 1, 2022 · 2 comments
Open

Refactor descrption of events #2

paulmillar opened this issue Oct 1, 2022 · 2 comments

Comments

@paulmillar
Copy link
Member

In #1, @pskopnik observed that "the ancillary lines match the format of the progress markers"

@pskopnik further writes:

I propose describing the text/perf-marker-stream format as a stream of individual reports.
Each report is "opened" by a single line - indicating the report's type - followed by a list of properties in the Key . ":" . Value format (with optional spaces) and then finished by a single line containing End.
There would be a Performance Marker Report type, starting with a Perf Marker line and a Summary Report type starting either with Success or Failure .*.
Properties and their semantics are then clearly related to the report type.

Of course this primarily affects naming and structure of the specification, the only effect on the wire protocol would be to (optionally?) add an End line to conclude the summary report.

This simplifies the description. Therefore, I think updating the overall description, according to this description is generally "a good idea", but I think care must be taken that we remain backwards compatible and acknowledge currently available formats.

@paulmillar
Copy link
Member Author

In this comment @pskopnik poses the question:

whether to add an End line to conclude the summary report/ancillary lines. I'm in favour of this change to bring the format in line with the progress markers.

@paulmillar
Copy link
Member Author

My concern here would be that we should try to avoid making changes that break existing servers. We should only do this if there's no other way.

So, we perhaps this could be added as an optional behaviour: a server MAY omit the final End for the final/summary event, or perhaps SHOULD.

The final End is somewhat redundant, as the end of the stream also identifies that no further information is coming (i.e., chunked encoding sending a zero-length block to indicate the end of the stream).

However, it's not complete redundant. The TCP connection could be prematurely closed, which risks that information was lost. This scenario would be identified by the HTTP client not receiving the final empty block from chunked encoding. However, the code processing the event stream might not have access to this information (whether the TCP connection was closed prematurely). In this case, sending End could help identify that information may have been lost.

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

No branches or pull requests

1 participant