ref(rules): Don't pass buffer to apply_delayed #69700
Conversation
github-actions
bot
added
the
Scope: Backend
| for rulegroup_to_event in rulegroup_to_events: | ||
| for rule_group in rulegroup_to_event.keys(): | ||
| rule_id, group_id = rule_group.split(":") | ||
| rule_id, group_id = rule_group.decode("utf-8").split(":") |
There was a problem hiding this comment.
Why do we need to decode here? If this is bytes, we should change the type of rulegroup_to_events. But it might be better to change get_hash to perform the decoding instead?
There was a problem hiding this comment.
Yeah it's bytes, this was wrong before probably because we were mocking the return value so it was missed.
There was a problem hiding this comment.
Ok I addressed this and also the fact that redis returns stuff in a list and we're always accessing the 0 index by doing the decoding and indexing in the buffer so it's cleaner to use later on
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #69700 +/- ##
===========================================
+ Coverage 57.90% 79.84% +21.94%
===========================================
Files 6460 6469 +9
Lines 287234 287692 +458
Branches 49525 49586 +61
===========================================
+ Hits 166333 229721 +63388
+ Misses 120504 57579 -62925
+ Partials 397 392 -5
|
| ) -> dict[str, str]: | ||
| key = self._make_key(model, field) | ||
| return self._execute_redis_operation(key, RedisOperation.HASH_GET_ALL) | ||
| redis_hash = self._execute_redis_operation(key, RedisOperation.HASH_GET_ALL) |
There was a problem hiding this comment.
I wonder if we should also have _execute_redis_operation only return a single value. We always either make one call in the pipeline, or a second call to set expire, but I don't know that we ever care about the result of that second call?
Might be a nicer interface to just return the value of the operation you requested
Follow up to comments on #69167 to not pass the Redis buffer to
apply_delayed, and also a minor comment to rename a function.