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

chore(web-server): reconnect window can be manually set but keeps a 60s default value #10754

Merged
merged 5 commits into from
Jan 30, 2025

Conversation

rafaelromcar-parabol
Copy link
Contributor

Description

Allows setting a custom reconnect window for the web server, so it can gracefully stop in environments where 60s grace period isn't allowed.

Demo

@rafaelromcar-parabol rafaelromcar-parabol changed the title draft: chore(web-server): reconnect window can be manually set but keeps a 60s default value chore(web-server): reconnect window can be manually set but keeps a 60s default value Jan 30, 2025
@@ -34,10 +34,13 @@ tracer.init({
tracer.use('ioredis').use('http').use('pg')

process.on('SIGTERM', async (signal) => {
const RECONNECT_WINDOW = process.env.WEB_SERVER_RECONNECT_WINDOW
? parseInt(process.env.WEB_SERVER_RECONNECT_WINDOW, 10) * 1000
: 60_000 // ms
Copy link
Contributor

Choose a reason for hiding this comment

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

-1 We should move this top level. That way it's evaluated already on first importing the file and a NUMBER_FORMAT_EXCEPTION would interrupt startup. It's better than finding invalid environment variables only at runtime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree! Where is that? 🤔

Copy link
Contributor

@Dschoordsch Dschoordsch Jan 30, 2025

Choose a reason for hiding this comment

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

Just at the top of the file is fine, l27.

.env.example Outdated
@@ -54,6 +54,10 @@ FILE_STORE_PROVIDER='local'
# Google Cloud credentials are required to use FILE_STORE_PROVIDER='gcs'
# GOOGLE_GCS_BUCKET='BUCKET_NAME'

# WEB SERVER
# Time window, in seconds, used by the web server to kick its users off. If empty, it has a default value.
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 Maybe mention the default value? "If empty it defaults to 60s."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I wanted to avoid that, just in case we changed the default value one day. That said, minor change and clearer information so I'm adding that

Copy link
Contributor

@Dschoordsch Dschoordsch left a comment

Choose a reason for hiding this comment

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

I forgot that JavaScript does not throw in parseInt. So validation would involve a bit more. It's ok.

@rafaelromcar-parabol rafaelromcar-parabol merged commit da37f80 into master Jan 30, 2025
6 checks passed
@rafaelromcar-parabol rafaelromcar-parabol deleted the chore/ws-reconnect-window branch January 30, 2025 15:01
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.

2 participants