Skip to content
This repository has been archived by the owner on Jul 28, 2022. It is now read-only.

Configured timeout to clear Symfony cache #115

Merged
merged 1 commit into from Sep 2, 2016

Conversation

mremi
Copy link
Member

@mremi mremi commented Aug 31, 2016

I am targetting this branch, because this is BC.

Changelog

### Added
- Added the possibility to configure the timeout to clear the Symfony cache.

Subject

The timeout to clear the Symfony cache was hard-coded (2 seconds), I add some configuration to be able to tweak it as you want.

*/
public function __construct(RouterInterface $router, Filesystem $filesystem, $cacheDir, $token, $phpCodeCacheEnabled, array $types, array $servers)
public function __construct(RouterInterface $router, Filesystem $filesystem, $cacheDir, $token, $phpCodeCacheEnabled, array $types, array $servers, array $timeout)
Copy link
Contributor

Choose a reason for hiding this comment

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

There are more and more arguments here some of which are options, the constructor is getting big… maybe we should consider using the options resolver (in another PR)?

@mremi
Copy link
Member Author

mremi commented Aug 31, 2016

@OskarStark @greg0ire up to date according to your comments

*/
public function __construct(RouterInterface $router, Filesystem $filesystem, $cacheDir, $token, $phpCodeCacheEnabled, array $types, array $servers)
public function __construct(RouterInterface $router, Filesystem $filesystem, $cacheDir, $token, $phpCodeCacheEnabled, array $types, array $servers, array $timeouts)
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding a mandatory argument is a BC-break

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I added the default values in the configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

So we're considering this class is internal and will never be used directly?

Copy link
Contributor

Choose a reason for hiding this comment

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

@soullivaneuh , can you comment on this?

Copy link
Member

Choose a reason for hiding this comment

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

The service would not be BC break but the class will be.

To have a perfect BC, we should make it optional with a warning like said on https://github.com/sonata-project/SonataCacheBundle/pull/115/files#r76977312.

@mremi
Copy link
Member Author

mremi commented Sep 1, 2016

@OskarStark @greg0ire @soullivaneuh up to date according to your new comments ;)

{
if (!$timeouts) {
@trigger_error('The argument "timeouts" is missing, please inject it.', E_USER_DEPRECATED);
Copy link
Contributor

Choose a reason for hiding this comment

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

"The "timeouts" argument is available since 3.x and will become mandatory in 4.0, please provide it"

->info(<<<'INFO'
Timeout value specifying the amount of seconds that an output function
blocks because flow control prevents data from being sent
INFO
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice heredoc usage.

@greg0ire
Copy link
Contributor

greg0ire commented Sep 1, 2016

LGTM, good job!

@OskarStark
Copy link
Member

Thank you @mremi 👍

@OskarStark OskarStark merged commit 988c8bb into sonata-project:2.x Sep 2, 2016
@mremi
Copy link
Member Author

mremi commented Sep 2, 2016

\o/ :) When do you think tag a new version?

@soullivaneuh
Copy link
Member

@mremi and you got it: https://github.com/sonata-project/SonataCacheBundle/releases/tag/2.3.0

Thanks for your contributions. 👍

{
if (!$timeouts) {
@trigger_error('The "timeouts" argument is available since 3.x and will become mandatory in 4.0, please provide it.', E_USER_DEPRECATED);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should have been 2.x and 3.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants