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.

Sign up for GitHub

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

Jump to bottom

Add redis as an (optional) session store backend #3450

Closed
wants to merge 3 commits into from
Closed

Add redis as an (optional) session store backend #3450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b2eb1d4
Add redis as an (optional) session store backend
djmaze Mar 16, 2021
ee307d0
Migrate session node module to multi-store backend
djmaze Mar 16, 2021
7890e0d
Fix Rubocop offenses in session store files
djmaze Mar 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ gem 'delayed_job_active_record'
# core - websocket
gem 'em-websocket'
gem 'eventmachine'
gem 'redis'

# core - password security
gem 'argon2'
Expand Down
2 changes: 2 additions & 0 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ GEM
rb-inotify (0.10.0)
ffi (~> 1.0)
rchardet (1.8.0)
redis (4.2.5)
regexp_parser (1.8.2)
rest-client (2.0.2)
http-cookie (>= 1.0.2, < 2.0)
Expand Down Expand Up @@ -660,6 +661,7 @@ DEPENDENCIES
rails-observers
rb-fsevent
rchardet (>= 1.8.0)
redis
rspec-rails
rszr (= 0.5.2)
rubocop
Expand Down
3 changes: 3 additions & 0 deletions config/application.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ class Application < Rails::Application
# define cache store
config.cache_store = :file_store, Rails.root.join('tmp', "cache_file_store_#{Rails.env}")
Copy link
Member

Choose a reason for hiding this comment

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

In container context i've used memcached for cache_store.
See: https://github.com/zammad/zammad-docker-compose/blob/master/containers/zammad/docker-entrypoint.sh#L58

Could we use redis here too to have a single solution for all file based caches and sessions stores?

Copy link
Contributor

Choose a reason for hiding this comment

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

Redis is actually our preferred long term solution 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my point of view, yes, that would make sense.

Copy link
Member

Choose a reason for hiding this comment

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

Would you mind to extend the pr for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure that belongs here because this is not about the cache store.

Also, you could probably just replace the :file_store with :redis_store in your docker image similar to how it is currently done with memcached. (Which is of course ugly and should be handled differently.)

Also, switching both cache and session store to Redis might bring a new class of problems, so we should probably also think about how to prevent those.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. We should limit the scope of this change to a minimum and instead iterate faster with smaller changes.


# define websocket session store
config.websocket_session_store = ENV['REDIS_URL'] ? :redis : :file

# default preferences by permission
config.preferences_default_by_permission = {
'ticket.agent' => {
Expand Down