Skip to content

[cache] Bump RedisAdapter default timeout to 5s #20991

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
Dec 27, 2016

Conversation

Nicofuma
Copy link
Contributor

Q A
Branch? 3.2
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Bump RedisAdapter timeout to 5s because (at least with Predis) 0 means 0s

@Nicofuma Nicofuma changed the title Patch 6 [cache] Bump RedisAdapter default timeout to 5s Dec 19, 2016
@Nicofuma
Copy link
Contributor Author

There is also an issue with read_timeout as predis uses read_write_timeout (and again, with predis, 0 means 0s), but just renaming the option would be a BC break

@@ -61,7 +61,7 @@ public function __construct($redisClient, $namespace = '', $defaultLifetime = 0)
* @param string $dsn
* @param array $options See self::$defaultConnectionOptions
*
* @throws InvalidArgumentException When the DSN is invalid.
* @throws InvalidArgumentException When the DSN is invalid
Copy link
Contributor

Choose a reason for hiding this comment

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

This CS fix should target 3.1, so it does not belong to the scope of this PR.

Copy link
Member

Choose a reason for hiding this comment

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

I would just revert this change as fabbot is unfortunately broken right now. PHP-CS-Fixer 2 is doing things we don't want.

Bump RedisAdapter timeout to 5s because (at least with Predis) 0 means 0s
@nicolas-grekas
Copy link
Member

Thank you @Nicofuma.

@nicolas-grekas nicolas-grekas merged commit eaca2a4 into symfony:3.1 Dec 27, 2016
nicolas-grekas added a commit that referenced this pull request Dec 27, 2016
This PR was merged into the 3.1 branch.

Discussion
----------

[cache] Bump RedisAdapter default timeout to 5s

| Q             | A
| ------------- | ---
| Branch?       | 3.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Bump RedisAdapter timeout to 5s because (at least with Predis) 0 means 0s

Commits
-------

eaca2a4 [cache] Bump RedisAdapter timeout to 5s
This was referenced Jan 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants