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.

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

Added configurable prefixes for Redis Tracker and Lock stores #6500

Merged
merged 36 commits into from
Nov 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
5e73237
Added configurable prefixes for Redis Tracker and Lock stores
bjbredis Aug 27, 2020
81ec8ae
Testing fixes
bjbredis Aug 27, 2020
961b449
Docs and test fix
bjbredis Aug 27, 2020
da21e2e
Docs
bjbredis Aug 27, 2020
c532922
Formatting
bjbredis Aug 27, 2020
959edb8
Changelog added.
bjbredis Aug 27, 2020
f0f6970
Cleanup validation logic.
bjbredis Aug 27, 2020
c1c3eb8
Formatting
bjbredis Aug 27, 2020
397dc1c
Post-linting changes.
bjbredis Aug 28, 2020
035c669
Fixed a test.
bjbredis Aug 28, 2020
d311713
Merge branch 'master' into add-prefix-redis-lockstore-trackerstore
bjbredis Aug 28, 2020
a7722c2
Update changelog/6498.improvement.md
bjbredis Sep 8, 2020
570b55a
Update docs/docs/lock-stores.mdx
bjbredis Sep 8, 2020
44dc016
Update data/test_endpoints/example_endpoints.yml
bjbredis Sep 8, 2020
80937c0
Added configurable prefixes for Redis Tracker and Lock stores
bjbredis Aug 27, 2020
0a07e11
Update docs/docs/lock-stores.mdx
bjbredis Oct 7, 2020
36d31f8
Fix wording in improvements
bjbredis Oct 7, 2020
56a3657
.
bjbredis Oct 7, 2020
c76ee9f
fixing docs
bjbredis Oct 7, 2020
83ed792
Update docs/docs/tracker-stores.mdx
bjbredis Oct 7, 2020
9fd4dad
Update docs/docs/tracker-stores.mdx
bjbredis Oct 7, 2020
322af40
Update docs/docs/tracker-stores.mdx
bjbredis Oct 7, 2020
dd63ec5
update docs/docs/tracker-stores.mdx
bjbredis Oct 7, 2020
703cfc4
Update rasa/core/tracker_store.py
bjbredis Oct 7, 2020
b4f60f3
Update rasa/core/tracker_store.py
bjbredis Oct 7, 2020
efac04b
Update rasa/core/lock_store.py
bjbredis Oct 7, 2020
93c3fdc
Update rasa/core/lock_store.py
bjbredis Oct 7, 2020
f7942d7
fixed tests
bjbredis Oct 29, 2020
19645bc
merge fixes
bjbredis Nov 2, 2020
0329fb8
more merge fixes
bjbredis Nov 3, 2020
4ff7345
Merge branch 'master' into add-prefix-redis-lockstore-trackerstore
bjbredis Nov 3, 2020
b37f130
lint fixes
bjbredis Nov 3, 2020
579e662
Merge branch 'master' into add-prefix-redis-lockstore-trackerstore
bjbredis Nov 3, 2020
98da0cf
flake8 linting
bjbredis Nov 3, 2020
1ee88d2
Merge branch 'add-prefix-redis-lockstore-trackerstore' of https://git…
bjbredis Nov 3, 2020
7377dc6
formatting changes from Black
bjbredis Nov 3, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog/6498.improvement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Adding configurable prefixes to Redis [Tracker](./tracker-stores.mdx) and [Lock Stores](./lock-stores.mdx) so that a single Redis instance (and logical DB) can support multiple conversation trackers and locks.
By default, conversations will be prefixed with `tracker:...` and all locks prefixed with `lock:...`. Additionally, you can add an alphanumeric-only `prefix: value` in `endpoints.yml` such that keys in redis will take the form `value:tracker:...` and `value:lock:...` respectively.
1 change: 1 addition & 0 deletions data/test_endpoints/example_endpoints.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ tracker_store:
port: 6379
db: 0
password: password
key_prefix: conversation
record_exp: 30000
# example of mongoDB external tracker store config
#tracker_store:
Expand Down
6 changes: 5 additions & 1 deletion docs/docs/lock-stores.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ address the same node when sending messages for a given conversation ID.
port: <port of your redis instance, usually 6379>
password: <password used for authentication>
db: <number of your database within redis, e.g. 0>
key_prefix: <alphanumeric value to prepend to lock store keys>
```

3. To start the Rasa Core server using your Redis backend, add the `--endpoints`
Expand All @@ -76,10 +77,13 @@ address the same node when sending messages for a given conversation ID.

* `db` (default: `1`): The number of your redis database

* `key_prefix` (default: `None`): The prefix to prepend to lock store keys. Must
be alphanumeric

* `password` (default: `None`): Password used for authentication
(`None` equals no authentication)

* `use_ssl` (default: `False`): Whether or not the communication is encrypted

* `socket_timeout` (default: `10`): Time in seconds after which an
error is raised if Redis doesn't answer
error is raised if Redis doesn't answer
10 changes: 5 additions & 5 deletions docs/docs/tracker-stores.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,12 @@ To set up Rasa Open Source with Redis the following steps are required:
type: redis
url: <url of the redis instance, e.g. localhost>
port: <port of your redis instance, usually 6379>
key_prefix: <alphanumeric value to prepend to tracker store keys>
db: <number of your database within redis, e.g. 0>
password: <password used for authentication>
use_ssl: <whether or not the communication is encrypted, default `false`>
```

3. To start the Rasa server using your configured Redis instance,
add the `--endpoints` flag, e.g.:

```bash
rasa run -m models --endpoints endpoints.yml
```
Expand All @@ -207,18 +205,20 @@ To set up Rasa Open Source with Redis the following steps are required:
url: <url of the redis instance, e.g. localhost>
port: <port of your redis instance, usually 6379>
db: <number of your database within redis, e.g. 0>
key_prefix: <alphanumeric value to prepend to tracker store keys>
password: <password used for authentication>
use_ssl: <whether or not the communication is encrypted, default `false`>
```

#### Configuration Parameters

* `url` (default: `localhost`): The url of your redis instance

* `port` (default: `6379`): The port which redis is running on

* `db` (default: `0`): The number of your redis database

* `key_prefix` (default: `None`): The prefix to prepend to tracker store keys. Must
be alphanumeric

* `password` (default: `None`): Password used for authentication
(`None` equals no authentication)

Expand Down
28 changes: 25 additions & 3 deletions rasa/core/lock_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ def _get_lock_lifetime() -> int:
LOCK_LIFETIME = _get_lock_lifetime()
DEFAULT_SOCKET_TIMEOUT_IN_SECONDS = 10

DEFAULT_REDIS_LOCK_STORE_KEY_PREFIX = "lock:"


# noinspection PyUnresolvedReferences
class LockError(RasaException):
Expand Down Expand Up @@ -199,6 +201,7 @@ def __init__(
db: int = 1,
password: Optional[Text] = None,
use_ssl: bool = False,
key_prefix: Optional[Text] = None,
socket_timeout: float = DEFAULT_SOCKET_TIMEOUT_IN_SECONDS,
) -> None:
"""Create a lock store which uses Redis for persistence.
Expand All @@ -211,6 +214,8 @@ def __init__(
password: The password which should be used for authentication with the
Redis database.
use_ssl: `True` if SSL should be used for the connection to Redis.
key_prefix: prefix to prepend to all keys used by the lock store. Must be
alphanumeric.
socket_timeout: Timeout in seconds after which an exception will be raised
in case Redis doesn't respond within `socket_timeout` seconds.
"""
Expand All @@ -224,19 +229,36 @@ def __init__(
ssl=use_ssl,
socket_timeout=socket_timeout,
)

self.key_prefix = DEFAULT_REDIS_LOCK_STORE_KEY_PREFIX
if key_prefix:
logger.debug(f"Setting non-default redis key prefix: '{key_prefix}'.")
self._set_key_prefix(key_prefix)

super().__init__()

def _set_key_prefix(self, key_prefix: Text) -> None:
if isinstance(key_prefix, str) and key_prefix.isalnum():
self.key_prefix = key_prefix + ":" + DEFAULT_REDIS_LOCK_STORE_KEY_PREFIX
else:
logger.warning(
f"Omitting provided non-alphanumeric redis key prefix: '{key_prefix}'. Using default '{self.key_prefix}' instead."
)

def _get_key_prefix(self) -> Text:
return self.key_prefix

def get_lock(self, conversation_id: Text) -> Optional[TicketLock]:
serialised_lock = self.red.get(conversation_id)
serialised_lock = self.red.get(self.key_prefix + conversation_id)
if serialised_lock:
return TicketLock.from_dict(json.loads(serialised_lock))

def delete_lock(self, conversation_id: Text) -> None:
deletion_successful = self.red.delete(conversation_id)
deletion_successful = self.red.delete(self.key_prefix + conversation_id)
self._log_deletion(conversation_id, deletion_successful)

def save_lock(self, lock: TicketLock) -> None:
self.red.set(lock.conversation_id, lock.dumps())
self.red.set(self.key_prefix + lock.conversation_id, lock.dumps())


class InMemoryLockStore(LockStore):
Expand Down
27 changes: 24 additions & 3 deletions rasa/core/tracker_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@
POSTGRESQL_DEFAULT_MAX_OVERFLOW = 100
POSTGRESQL_DEFAULT_POOL_SIZE = 50

# default value for key prefix in RedisTrackerStore
DEFAULT_REDIS_TRACKER_STORE_KEY_PREFIX = "tracker:"


class TrackerStore:
"""Class to hold all of the TrackerStore classes"""
Expand Down Expand Up @@ -305,6 +308,7 @@ def __init__(
password: Optional[Text] = None,
event_broker: Optional[EventBroker] = None,
record_exp: Optional[float] = None,
key_prefix: Optional[Text] = None,
use_ssl: bool = False,
**kwargs: Dict[Text, Any],
) -> None:
Expand All @@ -314,8 +318,25 @@ def __init__(
host=host, port=port, db=db, password=password, ssl=use_ssl
)
self.record_exp = record_exp

self.key_prefix = DEFAULT_REDIS_TRACKER_STORE_KEY_PREFIX
if key_prefix:
logger.debug(f"Setting non-default redis key prefix: '{key_prefix}'.")
self._set_key_prefix(key_prefix)

super().__init__(domain, event_broker, **kwargs)

def _set_key_prefix(self, key_prefix: Text) -> None:
if isinstance(key_prefix, str) and key_prefix.isalnum():
self.key_prefix = key_prefix + ":" + DEFAULT_REDIS_TRACKER_STORE_KEY_PREFIX
else:
logger.warning(
f"Omitting provided non-alphanumeric redis key prefix: '{key_prefix}'. Using default '{self.key_prefix}' instead."
)

def _get_key_prefix(self) -> Text:
return self.key_prefix

def save(self, tracker, timeout=None):
"""Saves the current conversation state"""
if self.event_broker:
Expand All @@ -325,18 +346,18 @@ def save(self, tracker, timeout=None):
timeout = self.record_exp

serialised_tracker = self.serialise_tracker(tracker)
self.red.set(tracker.sender_id, serialised_tracker, ex=timeout)
self.red.set(self.prefix + tracker.sender_id, serialised_tracker, ex=timeout)

def retrieve(self, sender_id: Text) -> Optional[DialogueStateTracker]:
stored = self.red.get(sender_id)
stored = self.red.get(self.prefix + sender_id)
if stored is not None:
return self.deserialise_tracker(sender_id, stored)
else:
return None

def keys(self) -> Iterable[Text]:
"""Returns keys of the Redis Tracker Store"""
return self.red.keys()
return self.red.keys(self.prefix + "*")


class DynamoTrackerStore(TrackerStore):
Expand Down
53 changes: 52 additions & 1 deletion tests/core/test_lock_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@
from rasa.core.constants import DEFAULT_LOCK_LIFETIME
from rasa.shared.constants import INTENT_MESSAGE_PREFIX
from rasa.core.lock import TicketLock
from rasa.core.lock_store import InMemoryLockStore, LockError, LockStore, RedisLockStore
from rasa.core.lock_store import (
InMemoryLockStore,
LockError,
LockStore,
RedisLockStore,
DEFAULT_REDIS_LOCK_STORE_KEY_PREFIX,
)


class FakeRedisLockStore(RedisLockStore):
Expand All @@ -32,6 +38,8 @@ def __init__(self):
# added in redis==3.3.0, but not yet in fakeredis
self.red.connection_pool.connection_class.health_check_interval = 0

self.key_prefix = DEFAULT_REDIS_LOCK_STORE_KEY_PREFIX


def test_issue_ticket():
lock = TicketLock("random id 0")
Expand Down Expand Up @@ -287,3 +295,46 @@ async def test_redis_lock_store_timeout(monkeypatch: MonkeyPatch):
with pytest.raises(LockError):
async with lock_store.lock("some sender"):
pass


async def test_redis_lock_store_with_invalid_prefix(monkeypatch: MonkeyPatch):
import redis.exceptions

lock_store = FakeRedisLockStore()

prefix = "!asdf234 34#"
lock_store._set_key_prefix(prefix)
assert lock_store._get_key_prefix() == DEFAULT_REDIS_LOCK_STORE_KEY_PREFIX

monkeypatch.setattr(
lock_store,
lock_store.get_or_create_lock.__name__,
Mock(side_effect=redis.exceptions.TimeoutError),
)

with pytest.raises(LockError):
async with lock_store.lock("some sender"):
pass


async def test_redis_lock_store_with_valid_prefix(monkeypatch: MonkeyPatch):
import redis.exceptions

lock_store = FakeRedisLockStore()

prefix = "chatbot42"
lock_store._set_key_prefix(prefix)
assert (
lock_store._get_key_prefix()
== prefix + ":" + DEFAULT_REDIS_LOCK_STORE_KEY_PREFIX
)

monkeypatch.setattr(
lock_store,
lock_store.get_or_create_lock.__name__,
Mock(side_effect=redis.exceptions.TimeoutError),
)

with pytest.raises(LockError):
async with lock_store.lock("some sender"):
pass
37 changes: 37 additions & 0 deletions tests/core/test_tracker_stores.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
TrackerStore,
InMemoryTrackerStore,
RedisTrackerStore,
DEFAULT_REDIS_TRACKER_STORE_KEY_PREFIX,
SQLTrackerStore,
DynamoTrackerStore,
FailSafeTrackerStore,
Expand Down Expand Up @@ -148,6 +149,42 @@ def test_create_tracker_store_from_endpoint_config(default_domain: Domain):
assert isinstance(tracker_store, type(TrackerStore.create(store, default_domain)))


def test_redis_tracker_store_invalid_key_prefix(default_domain: Domain):

test_invalid_key_prefix = "$$ &!"

tracker_store = RedisTrackerStore(
domain=default_domain,
host="localhost",
port=6379,
db=0,
password="password",
key_prefix=test_invalid_key_prefix,
record_exp=3000,
)

assert tracker_store._get_key_prefix() == DEFAULT_REDIS_TRACKER_STORE_KEY_PREFIX


def test_redis_tracker_store_valid_key_prefix(default_domain: Domain):
test_valid_key_prefix = "spanish"

tracker_store = RedisTrackerStore(
domain=default_domain,
host="localhost",
port=6379,
db=0,
password="password",
key_prefix=test_valid_key_prefix,
record_exp=3000,
)

assert (
tracker_store._get_key_prefix()
== f"{test_valid_key_prefix}:{DEFAULT_REDIS_TRACKER_STORE_KEY_PREFIX}"
)


def test_exception_tracker_store_from_endpoint_config(
default_domain: Domain, monkeypatch: MonkeyPatch
):
Expand Down
3 changes: 3 additions & 0 deletions tests/shared/core/test_trackers.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ def __init__(self, _domain: Domain) -> None:
# added in redis==3.3.0, but not yet in fakeredis
self.red.connection_pool.connection_class.health_check_interval = 0

# Defined in RedisTrackerStore but needs to be added for the MockRedisTrackerStore
self.prefix = "tracker:"

TrackerStore.__init__(self, _domain)


Expand Down