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 timeout to gunicorn in heroku #1127

Merged
merged 1 commit into from
Jun 22, 2022
Merged

Conversation

mvandenburgh
Copy link
Member

@mvandenburgh mvandenburgh commented Jun 21, 2022

Downstreaming kitware-resonant/cookiecutter-resonant#190 from the cookiecutter.

This will help distinguish Heroku request timeouts from other errors that cause a SystemExit in sentry.

@mvandenburgh mvandenburgh merged commit 9c2babf into master Jun 22, 2022
@mvandenburgh mvandenburgh deleted the add-gunicorn-timeout branch June 22, 2022 18:07
@waxlamp
Copy link
Member

waxlamp commented Jun 22, 2022

What's a good way to test this? Maybe an endpoint with an artificial delay that we can introduce and then remove later?

@mvandenburgh
Copy link
Member Author

What's a good way to test this? Maybe an endpoint with an artificial delay that we can introduce and then remove later?

Yeah I think that's the only way we could reliably test this. We could add a time.sleep call to an endpoint and test in staging.

@waxlamp
Copy link
Member

waxlamp commented Jun 24, 2022

What's a good way to test this? Maybe an endpoint with an artificial delay that we can introduce and then remove later?

Yeah I think that's the only way we could reliably test this. We could add a time.sleep call to an endpoint and test in staging.

Can you file an issue and assign it to yourself?

@jjnesbitt
Copy link
Member

FWIW I think making a temporary endpoint and merging it to master to test staging is the wrong approach. You can create this endpoint and run gunicorn locally, which imo is sufficient.

@mvandenburgh
Copy link
Member Author

Oh yeah i think you're right. For some reason I thought we had to test this in Heroku because we're testing the Heroku timeout, but in reality we are replacing the Heroku 30-second timeout with a 25-second timeout built in to gunicorn

I'll test it locally with gunicorn, @waxlamp does that sound good?

@waxlamp
Copy link
Member

waxlamp commented Jun 24, 2022

Yes, good idea.

@dandibot
Copy link
Member

🚀 PR was released in v0.2.26 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants