Skip to content

Add native HTTP/2 support#1489

Merged
hekike merged 1 commit into
restify:masterfrom
hekike:feat/http2
Nov 2, 2017
Merged

Add native HTTP/2 support#1489
hekike merged 1 commit into
restify:masterfrom
hekike:feat/http2

Conversation

@hekike

@hekike hekike commented Sep 12, 2017

Copy link
Copy Markdown
Contributor

Please provide feedback in the comment section about this approach.

Issues

Related:

Add HTTP/2 support to restify

Changes

  • Adds HTTP/2 support to server
  • Uses patch method to decorate Request and Response objects
  • Example for HTTP/2 server with restify

TODO

How to run

At the moment you need to build Node.js with the latest master and using the --expose-http2 flag to run this branch.

@hekike hekike changed the title feat(http2): add HTTP/2 support [WIP] A native HTTP/2 support Sep 12, 2017
@hekike hekike changed the title [WIP] A native HTTP/2 support [WIP] Add native HTTP/2 support Sep 12, 2017
@retrohacker

Copy link
Copy Markdown
Contributor

image

@yunong

yunong commented Sep 12, 2017

Copy link
Copy Markdown
Contributor

🎉 I've assigned a few reviewers to look at this, but this is amazing work so far.

@hekike

hekike commented Oct 25, 2017

Copy link
Copy Markdown
Contributor Author

@DonutEspresso @yunong @jclulow @retrohacker Node.js v8.8.0 is out which means we have everything that's needed to ship this feature, see changelog:

  • no more conflict with the path getter
  • http2 is now exposed by default without the need for a flag
  • However http2 stability is still experimental in the Node core

I rebased this PR with the master and added a simple test case.
Do you want to wait more or can we ship it?

@yunong

yunong commented Oct 26, 2017

Copy link
Copy Markdown
Contributor

Looks great. What do you think about adding some documentation to the home page about how we support HTTP/2, but that it's still experimental? I think this would give us tremendous PR!

@yunong

yunong commented Oct 26, 2017

Copy link
Copy Markdown
Contributor

What happens if we're on a version of Node that doesn't support http2, does the current code handle this path?

@hekike

hekike commented Oct 27, 2017

Copy link
Copy Markdown
Contributor Author

@yunong I added an assert to the server.js http2 option.

The http2 related tests will be skipped with older versions:

HTTP2 module is not available
Node.js version >= v8.8.8 required, current: 6.9.4

OK: 0 assertions (4ms)

@hekike

hekike commented Oct 27, 2017

Copy link
Copy Markdown
Contributor Author

@yunong regarding docs and landing I can create some hands-on guide for restify's http2 support, some ideas:

@yunong

yunong commented Oct 27, 2017

Copy link
Copy Markdown
Contributor

@hekike That blog post is great, maybe we can link to some articles instead of having to maintain HTTP/2 specific documentation.

I was more thinking about just adding docs to say that we support HTTP/2 now, with some pointers to code snippets or examples, which you already have added.

@hekike

hekike commented Oct 30, 2017

Copy link
Copy Markdown
Contributor Author

@yunong I agree. What do you think, should we link the articles it in this PR?
I believe we could focus on the marketing after the feature is landed.

@yunong

yunong commented Oct 31, 2017

Copy link
Copy Markdown
Contributor

Yeah we should land and then follow up with some docs :)

@hekike hekike force-pushed the feat/http2 branch 2 times, most recently from fcdb387 to 3072e38 Compare November 1, 2017 10:37
@hekike

hekike commented Nov 1, 2017

Copy link
Copy Markdown
Contributor Author

Breaks because of: #1545

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants