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

increase timeout to 15mins #231

Closed
wants to merge 1 commit into from
Closed

Conversation

katdrobnjakovic
Copy link

@KnVerey
Copy link
Contributor

KnVerey commented Dec 21, 2017

Looking at the linked issue, is this high timeout something you want generically for all Redis resources, or is it a small minority that are expected to take this long? I ask because in #225 we were discussing the possibility of adding a generic feature that would enable you to override the timeout on any individual resource via an annotation.

@katdrobnjakovic
Copy link
Author

Looking at the linked issue, is this high timeout something you want generically for all Redis resources, or is it a small minority that are expected to take this long?

Thanks for reading the linked PR 👍 We have only seen this timeout happen with the iq_* app redis resources; however, we are okay with this being global for all redis resources for now to solve the issues they are seeing.

I ask because in #225 we were discussing the possibility of adding a generic feature that would enable you to override the timeout on any individual resource via an annotation.

What is the timeframe for #225 to be shipped? We would definitely change to only overriding an individual resource using the annotation when that PR has shipped 👍

@KnVerey
Copy link
Contributor

KnVerey commented Dec 21, 2017

Nobody had started it, but I expect it to be quite easy to implement, so I'll whip up a PR now.

@KnVerey
Copy link
Contributor

KnVerey commented Dec 21, 2017

Opened that PR here: #232. PTAL. As mentioned there, a third option is to make the Redis class use progressDeadlineSeconds if available, and set that to a really high value for the problematic resource.

@katdrobnjakovic
Copy link
Author

Okay, Ill take a look. At first glance #232 is addressing our problem in a better way (individual resource increase of the timeout rather than global) so I will close this PR and focus on reviewing #232.

@katdrobnjakovic katdrobnjakovic deleted the increase-timeout branch December 22, 2017 14:57
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.

None yet

2 participants