Refactor Redis keys to avoid duplicate string literals#309
Refactor Redis keys to avoid duplicate string literals#309vishesh10 wants to merge 3 commits intocloseio:masterfrom
Conversation
tests/test_base.py
Outdated
| Worker(self.tiger).run(once=True) | ||
| self._ensure_queues(queued={"default": 0}) | ||
| assert not self.conn.exists("t:task:%s" % task["id"]) | ||
| assert not self.conn.exists(f"{REDIS_PREFIX}:{TASK}:%s" % task["id"]) |
There was a problem hiding this comment.
I would keep these literals unchanged in tests to make it clearer what these keys look like.
neob91-close
left a comment
neob91-close
left a comment
There was a problem hiding this comment.
This seems to almost only change tests (which we probably don't want to change) and not the literals in the actual feature code.
|
Thanks for the review. |
|
@neob91-close, |
tasktiger/constants.py
Outdated
| EXECUTIONS_COUNT = "executions_count" | ||
|
|
||
|
|
||
| QUEUED = "queued" |
There was a problem hiding this comment.
These are already defined in tasktiger._internal.
What's more, these definitions aren't even used.
We should get rid of them.
tasktiger/constants.py
Outdated
| @@ -0,0 +1,11 @@ | |||
| # constants pertaining to Redis keys | |||
| REDIS_PREFIX = "t" | |||
There was a problem hiding this comment.
This isn't used, nor is it needed (it's configurable, and the default value is defined in tasktiger.tasktiger.
tasktiger/constants.py
Outdated
| @@ -0,0 +1,11 @@ | |||
| # constants pertaining to Redis keys | |||
| REDIS_PREFIX = "t" | |||
| TASK = "task" | |||
There was a problem hiding this comment.
Can we move these definitions to tasktiger._internal for consistency?
neob91-close
left a comment
neob91-close
left a comment
There was a problem hiding this comment.
There's some more changes necessary.
By the way, sorry it took me so long to get back to you.
vishesh10
force-pushed
the
redis_keys
branch
from
873d95d to
b5d0ffe
Compare
|
Hi @neob91-close , |
2 participants
Description
Replaced string literals in the Redis keys to constants.
Resolves #255