server: Configurable websocket keep alive interval #6092

v0d1ch · 2020-10-28T21:52:57Z

Accept new server flag --websocket-keepalive to control keep-alive interval

close #3539

hasura-bot · 2020-10-28T21:53:00Z

Beep boop! 🤖

Hey @v0d1ch, thanks for your PR!

One of my human friends will review this PR and get back to you as soon as possible.

Stay awesome! 😎

CLAassistant · 2020-10-28T21:53:02Z

All committers have signed the CLA.

abooij · 2020-10-29T08:44:47Z

@v0d1ch If you intend this PR to be reviewed, then please look at the failing test cases - I believe them to be caused by the new --websocket-keepalive CLI flag being required rather than optional.

If you're still working on this, you can also keep this PR in a "draft" state for now - click "Convert to draft" under the "Reviewers" column on the right-hand side.

netlify · 2020-10-29T21:21:46Z

Deploy preview for hasura-docs ready!

Built with commit a11d929

https://deploy-preview-6092--hasura-docs.netlify.app

Accept new server flag --websocket-keepalive to control websockets keep-alive interval

v0d1ch · 2020-10-30T10:27:09Z

paging DR. @abooij ! This thing is ready for review now once you find the time.

abooij

Thanks for this PR, @v0d1ch. It looks like it's in quite a good state. I've added two potential simplifications.

If you want to adjust this PR, please append commits rather than force-pushing, so that I can see what you've changed from one round of reviews to the next. In this project we usually squash merge PRs, so it doesn't matter if the commit history of this PR gets a bit messy.

Are you doing this as part of Hacktoberfest?

I think the argument passing through some of the related functions could use some love but I didn't want to go out of scope of the task. If you want this handled just create the issue and I'd be happy to help out.

Yes, you're absolutely right, also for not going out of scope. I believe some of the WSServerEnv construction/destruction code can be vastly simplified by making better use of RecordWildCards, which is turned on globally. PRs are more than welcome! :-)

server/src-lib/Hasura/Server/Init/Config.hs

abooij · 2020-10-30T13:03:49Z

@tirumaraiselvan Could you please verify that you're happy with the CLI flags and environment variable naming? (And, as is tradition, the changelog.)

server/src-lib/Hasura/Server/Init.hs

server/src-lib/Hasura/Server/Init/Config.hs

abooij

Great, thanks a lot for this, @v0d1ch!

@tirumaraiselvan, could you please check that you are happy with the new flag --websockets-keepalive, and the changelog?

…erval

tirumaraiselvan · 2020-11-03T09:37:25Z

CHANGELOG.md

 - server: accept only non-negative integers for batch size and refetch interval (close #5653) (#5759)
 - server: fix bug which arised when renaming a table which had a manual relationship defined (close #4158)
 - server: limit the length of event trigger names (close #5786)
+- server: Configurable websocket keep-alive interval (fix #3539) 


Would be good to add the flag and env variable here.

hey @tirumaraiselvan changelog.md is updated 👍

tirumaraiselvan · 2020-11-03T10:11:41Z

CHANGELOG.md

 - server: accept only non-negative integers for batch size and refetch interval (close #5653) (#5759)
 - server: fix bug which arised when renaming a table which had a manual relationship defined (close #4158)
 - server: limit the length of event trigger names (close #5786)
+- server: Configurable websocket keep-alive interval. Add `--websockets-keepalive` command-line flag


Can we call it --websocket-keepalive as it is more natural and consistent with the ENV var?

tirumaraiselvan · 2020-11-03T12:00:51Z

server/src-lib/Hasura/Server/Init.hs

      , "log_level" J..= soLogLevel so
      , "plan_cache_options" J..= soPlanCacheOptions so
      , "websocket_compression_options" J..= show (WS.connectionCompressionOptions . soConnectionOptions $ so)
+      , "websockets_keep_alive" J..= show (soWebsocketKeepAlive so)


this should also be websocket_keep_alive :)

omg what else did I forgot :)

tirumaraiselvan

Approved changelog.

commented for a small typo if you could change it but no biggies.

Thanks!

abooij · 2020-11-03T12:04:43Z

@v0d1ch Thank you so much for accepting all our nitpicking, and fixing things without a single delay! I do promise we're usually more pragmatic :-)

…erval

v0d1ch · 2020-11-03T12:12:25Z

@v0d1ch Thank you so much for accepting all our nitpicking, and fixing things without a single delay! I do promise we're usually more pragmatic :-)

Naaah, it was a pleasure

…erval

abooij · 2020-11-03T15:13:24Z

(We just merged #6126 which adds some code linting requirements to the CI. These ended up raising warnings in this PR unrelated to your changes, thus blocking the merging of this work. I've taken the liberty to fix this on your PR right away.)

…erval

hasura-bot · 2020-11-03T17:04:52Z

Beep boop! 🤖

Awesome work @v0d1ch! All of us at Hasura ❤️ what you did.

Thanks again 🤗

Accept new server flag --websocket-keepalive to control websockets keep-alive interval Co-authored-by: Auke Booij <auke@hasura.io> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

v0d1ch force-pushed the 3539_configurable-websocket-keep-alive-interval branch from 4cc43ef to 944693f Compare October 28, 2020 22:01

v0d1ch requested a review from a team as a code owner October 28, 2020 22:01

v0d1ch force-pushed the 3539_configurable-websocket-keep-alive-interval branch from 944693f to ed966c7 Compare October 28, 2020 22:02

v0d1ch changed the title ~~3539 Configurable websocket keep alive interval~~ 3539 server: Configurable websocket keep alive interval Oct 28, 2020

v0d1ch changed the title ~~3539 server: Configurable websocket keep alive interval~~ server: Configurable websocket keep alive interval Oct 28, 2020

v0d1ch marked this pull request as draft October 29, 2020 09:12

v0d1ch force-pushed the 3539_configurable-websocket-keep-alive-interval branch 2 times, most recently from efbb189 to a3586dd Compare October 29, 2020 21:19

v0d1ch force-pushed the 3539_configurable-websocket-keep-alive-interval branch 2 times, most recently from d7336cd to b21a136 Compare October 29, 2020 21:26

3539 Configurable websocket keep alive interval

0000091

Accept new server flag --websocket-keepalive to control websockets keep-alive interval

v0d1ch force-pushed the 3539_configurable-websocket-keep-alive-interval branch from b21a136 to 0000091 Compare October 29, 2020 22:59

v0d1ch marked this pull request as ready for review October 29, 2020 23:00

abooij reviewed Oct 30, 2020

View reviewed changes

server/src-lib/Hasura/Server/Init/Config.hs Outdated Show resolved Hide resolved

server/src-lib/Hasura/Server/Init/Config.hs Outdated Show resolved Hide resolved

abooij added c/server Related to server community-contrib 🙏🏽 labels Oct 30, 2020

abooij reviewed Oct 30, 2020

View reviewed changes

server/src-lib/Hasura/Server/Init.hs Outdated Show resolved Hide resolved

PR review changes

7300865

abooij reviewed Oct 30, 2020

View reviewed changes

server/src-lib/Hasura/Server/Init.hs Outdated Show resolved Hide resolved

server/src-lib/Hasura/Server/Init/Config.hs Outdated Show resolved Hide resolved

v0d1ch added 3 commits October 30, 2020 15:15

Remove unneeded FromEnv instance for Integer

33e16ff

Remove leftover comments

dc87a35

Remove usage of secondsToDiffTime

a11d929

abooij added the auto-update-auto-merge label Nov 2, 2020

abooij approved these changes Nov 2, 2020

View reviewed changes

Merge branch 'master' into 3539_configurable-websocket-keep-alive-int…

325da4f

…erval

tirumaraiselvan reviewed Nov 3, 2020

View reviewed changes

Update changelog

23f309e

tirumaraiselvan reviewed Nov 3, 2020

View reviewed changes

Change to --websocket-keepalive flag as suggested

fef3603

tirumaraiselvan reviewed Nov 3, 2020

View reviewed changes

tirumaraiselvan approved these changes Nov 3, 2020

View reviewed changes

Update flag at one more place

3a80dbf

Merge branch 'master' into 3539_configurable-websocket-keep-alive-int…

c3325a9

…erval

Auke Booij added 2 commits November 3, 2020 16:07

Merge branch 'master' into 3539_configurable-websocket-keep-alive-int…

9bc5a03

…erval

fix hlint hints

dc464f5

Merge branch 'master' into 3539_configurable-websocket-keep-alive-int…

497bf57

…erval

abooij merged commit 81e836a into hasura:master Nov 3, 2020

rikinsk mentioned this pull request May 13, 2021

docs: document websocket keep alive interval #6915

Closed

TiagoJacobs mentioned this pull request Apr 23, 2023

Graphql Server - Add customized keep alive interval iMDT/bigbluebutton-summit-2k23#2

Open

server: Configurable websocket keep alive interval #6092

server: Configurable websocket keep alive interval #6092

Uh oh!

Conversation

v0d1ch commented Oct 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hasura-bot commented Oct 28, 2020

Uh oh!

CLAassistant commented Oct 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abooij commented Oct 29, 2020

Uh oh!

netlify bot commented Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

v0d1ch commented Oct 30, 2020

Uh oh!

abooij left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

abooij commented Oct 30, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abooij left a comment

Choose a reason for hiding this comment

Uh oh!

tirumaraiselvan Nov 3, 2020

Choose a reason for hiding this comment

Uh oh!

v0d1ch Nov 3, 2020

Choose a reason for hiding this comment

Uh oh!

tirumaraiselvan Nov 3, 2020

Choose a reason for hiding this comment

Uh oh!

tirumaraiselvan Nov 3, 2020

Choose a reason for hiding this comment

Uh oh!

v0d1ch Nov 3, 2020

Choose a reason for hiding this comment

Uh oh!

tirumaraiselvan left a comment

Choose a reason for hiding this comment

Uh oh!

abooij commented Nov 3, 2020

Uh oh!

v0d1ch commented Nov 3, 2020

Uh oh!

abooij commented Nov 3, 2020

Uh oh!

hasura-bot commented Nov 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

v0d1ch commented Oct 28, 2020 •

edited

Loading

CLAassistant commented Oct 28, 2020 •

edited

Loading

netlify bot commented Oct 29, 2020 •

edited

Loading

abooij left a comment •

edited

Loading