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.

Sign up for GitHub

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

Jump to bottom

Implement missing reason phrase in control and data API #37

Closed
nilp0inter opened this issue Oct 22, 2019 · 4 comments
Closed

Implement missing reason phrase in control and data API #37

nilp0inter opened this issue Oct 22, 2019 · 4 comments
Labels
discussion things to discuss about spec-compliance Behaviour detected that is not compliant with current spec.
Milestone
0.3.0

Comments

@nilp0inter
Copy link
Contributor

No description provided.

@nilp0inter nilp0inter added this to the 1.0.0 milestone Oct 22, 2019
@nilp0inter nilp0inter added the spec-compliance Behaviour detected that is not compliant with current spec. label Oct 22, 2019
@panchoh
Copy link
Contributor

panchoh commented Oct 23, 2019

I posit that we should perhaps not do this, in the light of this:
https://evertpot.com/http-2-finalized/ (look for 'reason phrase')
TL;DR: HTTP/2 does away with the reason phrase.

@panchoh panchoh mentioned this issue Oct 23, 2019
Closed
@panchoh panchoh added the discussion things to discuss about label Oct 23, 2019
@panchoh
Copy link
Contributor

panchoh commented Oct 23, 2019

Let's postpone this until we have a chance to discuss it.

@panchoh panchoh modified the milestones: 1.0.0, 1.1.0 Oct 23, 2019
@panchoh panchoh mentioned this issue Oct 31, 2019
Open
@nilp0inter nilp0inter modified the milestones: 1.1.0, 1.0.0 Nov 11, 2019
@nilp0inter
Copy link
Contributor Author

The custom reason phrases in the spec contains an important part of the message given to the client (the human explanation), so this can't be removed.
Given that is not likely that we use HTTP/2+ in the control/data API anyway, we decided to maintain the information in the reason phrase; if we finally decide to move to HTTP/2+ the messages will be moved to a custom header (not to the body because it adds complexity to the rest of the system).

@nilp0inter nilp0inter mentioned this issue Nov 11, 2019
Closed
@nilp0inter
Copy link
Contributor Author

Reason phrases where substituted by JSON payloads.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
discussion things to discuss about spec-compliance Behaviour detected that is not compliant with current spec.
Projects
None yet
Milestone
0.3.0
Development

No branches or pull requests
2 participants
@panchoh @nilp0inter