fix(integrations) removing redis watch and changing key to a daily redis hash key #54001
Conversation
github-actions
bot
added
the
Scope: Backend
|
|
||
| def _get(self): | ||
| """ | ||
| Returns the list of daily aggregate error counts. | ||
| """ | ||
| return [ | ||
| self._convert_obj_to_dict(obj) | ||
| self._convert_obj_to_dict(str(obj)) |
There was a problem hiding this comment.
Why are we stringifying the object here?
There was a problem hiding this comment.
json.loads() expects a str and redis object uses single quotes instead of double which json expects so I fix the quotes using .replace()
There was a problem hiding this comment.
I wonder if json.dumps() could help here
There was a problem hiding this comment.
Is hgetall not returning a python dict for you?
There was a problem hiding this comment.
hgetall does return a python dict so I removed this method :)
| for i in reversed(range(BUFFER_SIZE)): | ||
| with freeze_time(now - timedelta(days=i)): | ||
| cur = datetime.now().strftime("%Y-%m-%d") | ||
| buffer_key = self.integrationkey + cur |
There was a problem hiding this comment.
buffer_key = f"{self.integrationkey}:{cur}"?
| finally: | ||
| pipe.reset() | ||
| pipe.hincrby(buffer_key, count + "_count", 1) | ||
| pipe.expire(buffer_key, KEY_EXPIRY) |
There was a problem hiding this comment.
nit: you can maybe include an NX optional param for the expire command https://redis.io/commands/expire/ and only set the expire once. In practical terms, it probably won't matter since you're only refreshing the ttl for this day and then when the its the next day, the previous day hash won't ever be modified again.
There was a problem hiding this comment.
looks like NX also isn't supported
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #54001 +/- ##
=======================================
Coverage 79.53% 79.53%
=======================================
Files 4974 4974
Lines 210385 210380 -5
Branches 35832 35828 -4
=======================================
- Hits 167337 167334 -3
+ Misses 37901 37899 -2
Partials 5147 5147
|
| for i in reversed(range(IS_BROKEN_RANGE)): | ||
| cur = (now - timedelta(days=i)).strftime("%Y-%m-%d") | ||
| buffer_key = f"{self.integrationkey}:{cur}" | ||
| ret.append(self.client.hgetall(buffer_key)) |
There was a problem hiding this comment.
You could use a pipeline here, and in _get_all_from_buffer, since you already know all the key names in advance.
No description provided.