Skip to content

Set a timeout when purging caches #4121

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

Merged
merged 1 commit into from
Sep 17, 2018

Conversation

gisostallenberg
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets fixes #issuenum
Related issues/PRs #issuenum
License MIT
Documentation PR sulu/sulu-docs#prnum

What's in this PR?

Set a timeout when purging caches

Why?

The default behavior is to wait indefinitely (see guzzle documentation), which
may result in a very long request once the caches need to be cleared, but a long responding host is set in the config (see example configuration)

@alexander-schranz alexander-schranz added the Bug Error or unexpected behavior of already existing functionality label Aug 23, 2018
@alexander-schranz alexander-schranz added this to the Release 1.6 milestone Aug 23, 2018
@chirimoya chirimoya requested a review from danrot August 24, 2018 08:17
@danrot
Copy link
Contributor

danrot commented Aug 24, 2018

@chirimoya In theory this is OK for me, but I don't really have experience with what a sensible value for the timeout is.

@gisostallenberg When does the request fail now? After the max execution time of PHP? And in the current master branch you still have to add a line in the CHANGELOG.

@gisostallenberg
Copy link
Contributor Author

@danrot, @chirimoya I will change the value to 5 seconds. The reasoning is that Sulu should take less than 3 seconds to spin up, but in some cases this might be to rigid. I'll also provide a CHANGELOG line.

The default behavior is to wait indefinitely (see http://docs.guzzlephp.org/en/stable/request-options.html#connect-timeout), which
may result in a very long request once the caches need to be cleared, but a long responding host is set in the config (see https://github.com/sulu/sulu-minimal/blob/7887ff338657fa5148c56153030d835d0da02c49/app/Resources/webspaces/example.com.xml#L43)
@gisostallenberg
Copy link
Contributor Author

@danrot, @chirimoya I updated the PR as promised.

@danrot danrot merged commit 57422be into sulu:master Sep 17, 2018
@danrot
Copy link
Contributor

danrot commented Sep 17, 2018

@gisostallenberg thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Error or unexpected behavior of already existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants