Skip to content
This repository was archived by the owner on Jan 31, 2020. It is now read-only.

Redis auth parameter #66

Merged
merged 3 commits into from
Mar 9, 2016
Merged

Redis auth parameter #66

merged 3 commits into from
Mar 9, 2016

Conversation

sam-sandmann
Copy link
Contributor

No description provided.

@sam-sandmann sam-sandmann changed the title Feature/redis auth Redis auth parameter Feb 25, 2016
@ihortymoshenko
Copy link

+1 :)

@DrSchimke
Copy link
Contributor

👍

$client->connect($this->host, $this->port);

if ($this->auth) {
$client->auth($this->auth);
Copy link
Member

Choose a reason for hiding this comment

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

Please add return value check. If the authentication fails, IMO the test should return a Failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added exception for authorization check, but it's overhead, because Redis when password is incorrect throws RedisException and then test will return Failure.

@ihortymoshenko
Copy link

ping :)

@Thinkscape
Copy link
Member

I'm going to merge it, but I'm not satisfied with how missing extensions are inconsistently handled in checks. Checks are not supposed to throw exceptions. Even though the default Runner here, and in ZFTool, will catch all exceptions and convert them into new Failure() this is not an expected behavior.

Thinkscape added a commit that referenced this pull request Mar 9, 2016
@Thinkscape Thinkscape merged commit 7f48aa0 into zendframework:master Mar 9, 2016
@Thinkscape
Copy link
Member

I'll take care of the cleanup.

@ihortymoshenko
Copy link

@Thinkscape, please make a new tag

@Thinkscape
Copy link
Member

@ihortymoshenko
Copy link

@Thinkscape, thanks

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

Successfully merging this pull request may close these issues.

4 participants