Skip to content

Conversation

Winnie-Fred
Copy link
Contributor

I added timeout setting to gunicorn config which is configurable via environment variable.

Copy link
Owner

@nickjj nickjj left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the PR.


reload = bool(strtobool(os.getenv("WEB_RELOAD", "false")))

timeout = int(os.getenv("GUNICORN_TIMEOUT", 120))
Copy link
Owner

Choose a reason for hiding this comment

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

Can you rename this to WEB_TIMEOUT?

.env.example Outdated
#export WEB_RELOAD=false
export WEB_RELOAD=true

# GUNICORN_TIMEOUT sets the timeout for gunicorn workers, defaults to 120
Copy link
Owner

Choose a reason for hiding this comment

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

# Configure the timeout value in seconds for gunicorn.

@Winnie-Fred Winnie-Fred requested a review from nickjj June 28, 2024 21:42
.env.example Outdated
# seconds.
#export GUNICORN_TIMEOUT=
# Configure the timeout value in seconds for gunicorn.
#export WEB_TIMEOUT=
Copy link
Owner

@nickjj nickjj Jun 28, 2024

Choose a reason for hiding this comment

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

Given it defaults to 120, can you also set #export WEB_TIMEOUT=120 here so folks know what the default value is? Currently it's missing the 120 value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing.

@Winnie-Fred Winnie-Fred requested a review from nickjj June 29, 2024 05:30
@nickjj
Copy link
Owner

nickjj commented Jun 29, 2024

Looks good, can you roll up the commits to a single one with a message of "Add configurable gunicorn timeout value"?

@Winnie-Fred
Copy link
Contributor Author

Done, I have rolled up the commits.

Copy link
Owner

@nickjj nickjj left a comment

Choose a reason for hiding this comment

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

Looks good, thanks a lot for the PR.

@nickjj nickjj merged commit 515e325 into nickjj:main Jun 30, 2024
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