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

Add keepalive example #2688

Merged
merged 2 commits into from
Mar 14, 2024
Merged

Conversation

murgatroid99
Copy link
Member

This adds an example of configuring client and server keepalive options, modeled on the Go example. Note that ping enforcement settings are not included because that functionality is not implemented in Node.

I had to bump the minimum required grpc-js version to 1.10.2 because that is the version that added support for the grpc.max_connection_idle_ms server option.

examples/keepalive/README.md Outdated Show resolved Hide resolved
Comment on lines +3 to +4
This example illustrates how to set up client-side keepalive pings and
server-side keepalive pings and connection age and idleness settings.
Copy link

Choose a reason for hiding this comment

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

readability nit: "...set up client and server keepalive pings as well as connection..."

Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly, I'm not sure. I'm trying to convey that the keepalive ping settings are on both the client and the server, but the connection age and idleness settings are only on the server. The Go example uses this wording:

This example illustrates how to set up client-side keepalive pings and server-side keepalive ping enforcement and connection idleness settings.

Which is similar, but also omits the fact that it has server-side keepalive pings.

Copy link

Choose a reason for hiding this comment

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

Oh I see, on initial read this distinction did not come through, but makes sense now that you mentioned it. If you wanted to be super clear you go with:

"...how to set up keepalive pings on the client-side and keepalive pings, connection age and idleness setting on the server-side"

But I'll leave it up to you.

examples/keepalive/client.js Outdated Show resolved Hide resolved
@murgatroid99 murgatroid99 merged commit bcea4b4 into grpc:master Mar 14, 2024
8 of 10 checks passed
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

Successfully merging this pull request may close these issues.

2 participants