Skip to content
This repository has been archived by the owner on May 9, 2024. It is now read-only.

Redis #118

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Redis #118

wants to merge 4 commits into from

Conversation

jamesmorrison
Copy link
Member

@jamesmorrison jamesmorrison commented May 18, 2020

Description of the Change

Adding Redis object caching support

Alternate Designs

Benefits

Many projects are hosted on platforms using Redis rather than Memcached, adding support for Redis allows the object cache to be utilised and tested locally as well as on staging environments prior to a production release.

Possible Drawbacks

Verification Process

This was tested on a project locally, in conjunction with WP Redis and a standard configuration and showed the object cache was functioning correctly:

$redis_server = array(
	'host' => 'redis',
	'port' => 6379,
);

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Changelog Entry

Added Redis Object Cache support.

src/create.js Outdated Show resolved Hide resolved
@jeffpaul jeffpaul modified the milestones: 2.8.0, 2.9.0 Aug 10, 2020
@eugene-manuilov
Copy link
Collaborator

I understand that redis is important for some projects, but I think this PR is not ready for the release yet because we can't accept it as is. If we want to add redis support, we need to modify the create command to ask users which caching service they want to use and let them select between memcached and redis. @jamesmorrison do you mind to rebase your branch against release/2.8.0 and implement it?

Copy link
Collaborator

@eugene-manuilov eugene-manuilov left a comment

Choose a reason for hiding this comment

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

We need to do the following:

  1. rebase agains release/2.8.0 branch;
  2. ask users which caching service they want to use and let them to select between memcached and redis;
  3. update docker-composer config creation script to use caching service selected by the user;
  4. update init command to ask the same question and save user selection in the config file;
  5. add redis image to the images list, so it gets updated when we update images with image update command;
  6. update changelog.

@jeffpaul jeffpaul changed the base branch from master to develop January 26, 2021 20:35
@jeffpaul
Copy link
Member

@jamesmorrison are you able to help work through any of the 6 items that Eugene noted in his prior review comment?

@jamesmorrison
Copy link
Member Author

👋🏻 @jeffpaul - Yes I'd like to finish this up, it didn't slip my mind but I haven't found time to work on this yet.

I've recently completed a Node CLI course; and have been exploring Docker / Docker Compose a lot recently so it would be nice to see this through!

@eugene-manuilov eugene-manuilov removed this from the 2.9.0 milestone Sep 6, 2021
@jeffpaul jeffpaul added this to the Future Release milestone Jan 19, 2022
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