-
Notifications
You must be signed in to change notification settings - Fork 651
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
3722 embeds vizjson in redis #3733
Changes from all commits
5c0179e
4298da4
bae4073
2ef81b9
a88b1da
61ed801
6e288bf
b146146
9dd58d4
91e72cb
d79b966
2c53fe8
4f16361
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
# encoding: utf-8 | ||
|
||
class EmbedRedisCache | ||
|
||
# This needs to be changed whenever there're changes in the code that require invalidation of old keys | ||
VERSION = '1' | ||
|
||
|
||
def initialize(redis_cache = $tables_metadata) | ||
@redis = redis_cache | ||
end | ||
|
||
def get(visualization_id) | ||
key = key(visualization_id) | ||
value = redis.get(key) | ||
if value.present? | ||
return JSON.parse(value, symbolize_names: true) | ||
else | ||
return nil | ||
end | ||
rescue Redis::BaseError => exception | ||
Rollbar.report_exception(exception) | ||
nil | ||
end | ||
|
||
# Only public and public with link | ||
def set(visualization_id, response_headers, response_body) | ||
serialized = JSON.generate({headers: response_headers, | ||
body: response_body | ||
}) | ||
redis.setex(key(visualization_id), 24.hours.to_i, serialized) | ||
rescue Redis::BaseError => exception | ||
Rollbar.report_exception(exception) | ||
nil | ||
end | ||
|
||
def invalidate(visualization_id) | ||
redis.del key(visualization_id) | ||
rescue Redis::BaseError => exception | ||
Rollbar.report_exception(exception) | ||
nil | ||
end | ||
|
||
def key(visualization_id) | ||
"visualization:#{visualization_id}:embed:#{VERSION}" | ||
end | ||
|
||
|
||
private | ||
|
||
def redis | ||
@redis | ||
end | ||
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
require_relative '../table/privacy_manager' | ||
require_relative '../../../services/minimal-validation/validator' | ||
require_relative '../../../services/named-maps-api-wrapper/lib/named_maps_wrapper' | ||
require_relative '../../helpers/embed_redis_cache' | ||
|
||
# Every table has always at least one visualization (the "canonical visualization"), of type 'table', | ||
# which shares the same privacy options as the table and gets synced. | ||
|
@@ -610,18 +611,24 @@ def qualified_name(viewer_user=nil) | |
def invalidate_redis_cache | ||
self.class.redis_cache.del(redis_vizjson_key) | ||
self.class.redis_cache.del(redis_vizjson_key(true)) | ||
embed_redis_cache.invalidate(self.id) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you checked this is called on permission change? I've taken a quick look through Visualization::Member code regarding caches invalidation but it's messy. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a bug prior to these changes on permission changes, it is quite easy to reproduce #2407 I'll fix it afterwards (it must also affect varnish btw) |
||
end | ||
|
||
def self.redis_cache | ||
@@redis_cache ||= $tables_metadata | ||
end | ||
|
||
|
||
|
||
private | ||
|
||
attr_reader :repository, :name_checker, :validator | ||
attr_accessor :privacy_changed, :name_changed, :old_name, :permission_change_valid, :dirty | ||
|
||
def embed_redis_cache | ||
@embed_redis_cache ||= EmbedRedisCache.new($tables_metadata) | ||
end | ||
|
||
def calculate_vizjson(options={}) | ||
vizjson_options = { | ||
full: false, | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'm over-thinking, but after a second thought, if Redis went down or there were a error or connection issue this would fail and it wouldn't fallback to normal rendering, maybe this class should have error handling: reporting it but not propagating the exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also needed for vizjson cache, maybe I will make a separate branch for that that will conflict a little with the branch refactoring it out of old and new models :S
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if redis go down, don't worry, everything will go down