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

Updated redis session handler to support redis 5.0.x #2125

Merged
merged 1 commit into from
Aug 12, 2019
Merged

Updated redis session handler to support redis 5.0.x #2125

merged 1 commit into from
Aug 12, 2019

Conversation

tangix
Copy link
Contributor

@tangix tangix commented Aug 11, 2019

Fixes #2121

Description
Updated RedisHandler to support redis 5.0.x and php 7.3.x by removing deprecated calls and updating ping() handling. This while maintaining backward compatibility with redis 4.1.x and php 7.2.x

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@lonnieezell lonnieezell merged commit ea9bdb3 into codeigniter4:develop Aug 12, 2019
@MGatner
Copy link
Member

MGatner commented Aug 13, 2019

Redis tests all seem to be failing since this was merge.

@tangix
Copy link
Contributor Author

tangix commented Aug 13, 2019

The failed builds are happening down in the cache handler but for the same reason - the deprecated delete() call;
https://travis-ci.org/codeigniter4/CodeIgniter4/jobs/571193301
My commit addressed the issues in Sessionhandler: system/Session/Handlers/RedisHandler.php

@tangix
Copy link
Contributor Author

tangix commented Aug 13, 2019

I see in the travis log that the build infrastructure was changed between this PR and now - job 4904 was running PHP 7.2.16/7.3.2 and 4927 PHP 7.2.21/7.3.8 could something else has changed in the testing process as well, like error level?

@MGatner
Copy link
Member

MGatner commented Aug 13, 2019

You're totally right! Sorry for the false accusation. I think we'll need backup from @lonnieezell on this one.

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.

Redis
3 participants