search

vibe-d / vibe-http

Future vibe.d HTTP implementation
15 stars 9 forks source link

HTTP2 Settings struct #4

Closed GallaFrancesco closed 6 years ago

GallaFrancesco commented 6 years ago

A structure which holds the 6 settings parameters used during version negotiation between HTTP/1 and HTTP/2.

As we discussed, I copied above the involved code the relevant parts of the HTTP/2 RFC which in this case are section 3.2.1 and section 6.5.

I would like to discuss how should we manage the initialization of the values which are considered "unimited" by default. At the moment one has been set to the minimum required value and the other hasn't been initialized. We could also postpone them to when we find some optimal value for Vibe.d or avoid checking if the values are not received by the client (assuming we are considering a server context here).

s-ludwig commented 6 years ago

With regards to the unlimited value, the usual convention for vibe.d is to use T.max for that. But as written in the inline-comment, I'd agree that we probably should postpone a final decision until we have some real-world benchmarks.

s-ludwig commented 6 years ago

As for the RFC, it is indeed very convenient to have the full text. If there are no copyright issues involved, and the amount of comment text stays managable, we can keep it this way. Otherwise it would also be fine to just have the section number mentioned. At least I'll probably keep the RFC permanently open in a tab anyway.

GallaFrancesco commented 6 years ago

Thank you for the suggestions, I definitely needed them. I applied the changes above, setting HTTP2SettingValue.max for the unlimited values. I don't think copyright will be an issue as long as we use only essential parts of the RFC (which we can always remove or substitute with the corresponding section number while the library grows).

I'll try to remain coherent with the Vibe.d style guidelines from now on, of course any remark is greatly appreciated!