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

Allow to connect to redis by socket #18395

Merged
merged 3 commits into from Nov 24, 2017

Conversation

Projects
None yet
7 participants
@csthomas
Contributor

csthomas commented Oct 23, 2017

Summary of Changes

Further development of #18266

Inspired by #15822, #17528 and my own #16904

  1. Allow to connect to Redis by socket (cache and session handler).

    • check first character of supplied server host to check if it is a socket path or not.
  2. Fix for "CLI scripts print 'Application Instantiation Error' when redis cache enabled" but without breaking B/C.

    • use \JFactory::getApplication() only if needed, this way redis can be use on CLI script, see #16904.
  3. Add support for Redis Authentication in Session handler, the same as in cache handler.

  4. Add option to set/unset persistent connection in Session Redis handler.

More info:

Before

obraz

After

obraz

Testing Instructions

Test cache and session handler using Redis by socket.

Expected result

All works as expected.

Actual result

Redis by socket is not available.

Documentation Changes Required

Maybe

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Oct 28, 2017

@simbus82 can you please test this PR?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18395.

franz-wohlkoenig commented Oct 28, 2017

@simbus82 can you please test this PR?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18395.

@alikon

This comment has been minimized.

Show comment
Hide comment
@alikon

alikon Oct 29, 2017

Contributor

I have tested this item successfully on 39ae904


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18395.

Contributor

alikon commented Oct 29, 2017

I have tested this item successfully on 39ae904


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18395.

@alikon

This comment has been minimized.

Show comment
Hide comment
@alikon

alikon Oct 29, 2017

Contributor

#17528 can be closed in favour of this one

Contributor

alikon commented Oct 29, 2017

#17528 can be closed in favour of this one

@csthomas

This comment has been minimized.

Show comment
Hide comment
@csthomas

csthomas Oct 30, 2017

Contributor

@simbus82 Would you be interested in testing this PR?

Contributor

csthomas commented Oct 30, 2017

@simbus82 Would you be interested in testing this PR?

@simbus82

This comment has been minimized.

Show comment
Hide comment
@simbus82

simbus82 Oct 30, 2017

I have tested this item successfully on 39ae904

For me is OK! I have tested the socket and auth both for cache and session.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18395.

simbus82 commented Oct 30, 2017

I have tested this item successfully on 39ae904

For me is OK! I have tested the socket and auth both for cache and session.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18395.

@csthomas

This comment has been minimized.

Show comment
Hide comment
@csthomas

csthomas Oct 30, 2017

Contributor

Thank you all for testing

Contributor

csthomas commented Oct 30, 2017

Thank you all for testing

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Oct 30, 2017

Contributor

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18395.

Contributor

dgrammatiko commented Oct 30, 2017

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18395.

@joomla-cms-bot joomla-cms-bot added the RTC label Oct 30, 2017

@mbabker mbabker added this to the Joomla 3.8.3 milestone Nov 24, 2017

@mbabker mbabker merged commit 43ed2da into joomla:staging Nov 24, 2017

4 of 5 checks passed

continuous-integration/drone/pr the build failed
Details
JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound No violations found. Woof!

@joomla-cms-bot joomla-cms-bot removed the RTC label Nov 24, 2017

@csthomas csthomas deleted the csthomas:redis_improvement branch Nov 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment