From fa13dfc91b8f792e5bc0e5ff91d553b2ae30b4af Mon Sep 17 00:00:00 2001 From: vcidst Date: Wed, 20 Sep 2023 13:31:11 +0200 Subject: [PATCH 01/11] add username property --- docs/docs/lock-stores.mdx | 2 ++ rasa/core/lock_store.py | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/docs/docs/lock-stores.mdx b/docs/docs/lock-stores.mdx index 5889d953eac8..6f97a37e8e93 100644 --- a/docs/docs/lock-stores.mdx +++ b/docs/docs/lock-stores.mdx @@ -156,6 +156,8 @@ The `ConcurrentRedisLockStore` recreates the `TicketLock` from the persisted `Ti - `key_prefix` (default: `None`): The prefix to prepend to lock store keys. Must be alphanumeric + - `username` (default: `None`): Username used for authentication + - `password` (default: `None`): Password used for authentication (`None` equals no authentication) diff --git a/rasa/core/lock_store.py b/rasa/core/lock_store.py index b7d495b3f1f5..e8baff134a2e 100644 --- a/rasa/core/lock_store.py +++ b/rasa/core/lock_store.py @@ -200,6 +200,7 @@ def __init__( host: Text = "localhost", port: int = 6379, db: int = 1, + username: Optional[Text] = None, password: Optional[Text] = None, use_ssl: bool = False, ssl_certfile: Optional[Text] = None, @@ -215,6 +216,8 @@ def __init__( port: The port of the redis server. db: The name of the database within Redis which should be used by Rasa Open Source. + username: The username which should be used for authentication with the + Redis database. 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. @@ -232,6 +235,7 @@ def __init__( host=host, port=int(port), db=int(db), + username=username, password=password, ssl=use_ssl, ssl_certfile=ssl_certfile, From 0bfc8f94ef2d035d0d0367e3f3486c3b90d7eb10 Mon Sep 17 00:00:00 2001 From: vcidst Date: Wed, 20 Sep 2023 13:33:10 +0200 Subject: [PATCH 02/11] add changelog --- changelog/12835.improvement.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/12835.improvement.md diff --git a/changelog/12835.improvement.md b/changelog/12835.improvement.md new file mode 100644 index 000000000000..4869a372b594 --- /dev/null +++ b/changelog/12835.improvement.md @@ -0,0 +1 @@ +Added `username` to the connection parameters for `RedisLockStore` \ No newline at end of file From e780ff3e73d78fccbc1c8cf0fbddcf99597cd6ff Mon Sep 17 00:00:00 2001 From: vcidst Date: Wed, 20 Sep 2023 14:20:05 +0200 Subject: [PATCH 03/11] add tests and username in RedisTrackerStore --- changelog/12835.improvement.md | 2 +- docs/docs/tracker-stores.mdx | 2 ++ rasa/core/tracker_store.py | 2 ++ tests/core/test_lock_store.py | 44 ++++++++++++++++++++++++++++++- tests/core/test_tracker_stores.py | 2 ++ 5 files changed, 50 insertions(+), 2 deletions(-) diff --git a/changelog/12835.improvement.md b/changelog/12835.improvement.md index 4869a372b594..0fea8ac19d52 100644 --- a/changelog/12835.improvement.md +++ b/changelog/12835.improvement.md @@ -1 +1 @@ -Added `username` to the connection parameters for `RedisLockStore` \ No newline at end of file +Added `username` to the connection parameters for `RedisLockStore` and `RedisTrackerStore` \ No newline at end of file diff --git a/docs/docs/tracker-stores.mdx b/docs/docs/tracker-stores.mdx index f3ac71f4a2d1..f0485583c451 100644 --- a/docs/docs/tracker-stores.mdx +++ b/docs/docs/tracker-stores.mdx @@ -219,6 +219,8 @@ To set up Rasa with Redis the following steps are required: * `key_prefix` (default: `None`): The prefix to prepend to tracker store keys. Must be alphanumeric +* `username` (default: `None`): Username used for authentication + * `password` (default: `None`): Password used for authentication (`None` equals no authentication) diff --git a/rasa/core/tracker_store.py b/rasa/core/tracker_store.py index 2d38d22c26f3..adcabfc1388e 100644 --- a/rasa/core/tracker_store.py +++ b/rasa/core/tracker_store.py @@ -453,6 +453,7 @@ def __init__( host: Text = "localhost", port: int = 6379, db: int = 0, + username: Optional[Text] = None, password: Optional[Text] = None, event_broker: Optional[EventBroker] = None, record_exp: Optional[float] = None, @@ -470,6 +471,7 @@ def __init__( host=host, port=port, db=db, + username=username, password=password, ssl=use_ssl, ssl_keyfile=ssl_keyfile, diff --git a/tests/core/test_lock_store.py b/tests/core/test_lock_store.py index bba472696197..e182e3beb5ef 100644 --- a/tests/core/test_lock_store.py +++ b/tests/core/test_lock_store.py @@ -25,7 +25,10 @@ DEFAULT_REDIS_LOCK_STORE_KEY_PREFIX, ) from rasa.shared.exceptions import ConnectionException -from rasa.utils.endpoints import EndpointConfig +from rasa.utils.endpoints import EndpointConfig, read_endpoint_config + +from typing import Text +from rasa.shared.core.domain import Domain class FakeRedisLockStore(RedisLockStore): @@ -384,3 +387,42 @@ async def test_redis_lock_store_with_valid_prefix(monkeypatch: MonkeyPatch): with pytest.raises(LockError): async with lock_store.lock("some sender"): pass + + +def test_tracker_store_endpoint_config_loading(endpoints_path: Text): + cfg = read_endpoint_config(endpoints_path, "tracker_store") + + assert cfg == EndpointConfig.from_dict( + { + "type": "redis", + "url": "localhost", + "port": 6379, + "db": 0, + "username": "username", + "password": "password", + "timeout": 30000, + "use_ssl": True, + "ssl_keyfile": "keyfile.key", + "ssl_certfile": "certfile.crt", + "ssl_ca_certs": "my-bundle.ca-bundle", + } + ) + + +def test_create_lock_store_from_endpoint_config(domain: Domain, endpoints_path: Text): + store = read_endpoint_config(endpoints_path, "tracker_store") + tracker_store = RedisLockStore( + domain=domain, + host="localhost", + port=6379, + db=0, + username="username", + password="password", + record_exp=3000, + use_ssl=True, + ssl_keyfile="keyfile.key", + ssl_certfile="certfile.crt", + ssl_ca_certs="my-bundle.ca-bundle", + ) + + assert isinstance(tracker_store, type(LockStore.create(store, domain))) diff --git a/tests/core/test_tracker_stores.py b/tests/core/test_tracker_stores.py index e0175dad8881..811061ccedf4 100644 --- a/tests/core/test_tracker_stores.py +++ b/tests/core/test_tracker_stores.py @@ -143,6 +143,7 @@ def test_tracker_store_endpoint_config_loading(endpoints_path: Text): "url": "localhost", "port": 6379, "db": 0, + "username": "username", "password": "password", "timeout": 30000, "use_ssl": True, @@ -162,6 +163,7 @@ def test_create_tracker_store_from_endpoint_config( host="localhost", port=6379, db=0, + username="username", password="password", record_exp=3000, use_ssl=True, From c1a4bfe7f10b6cd1ff90c7b89aa049ad3409d1d4 Mon Sep 17 00:00:00 2001 From: vcidst Date: Thu, 21 Sep 2023 11:26:31 +0200 Subject: [PATCH 04/11] fixed spacing --- docs/docs/lock-stores.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/lock-stores.mdx b/docs/docs/lock-stores.mdx index 6f97a37e8e93..8f0a1b66a285 100644 --- a/docs/docs/lock-stores.mdx +++ b/docs/docs/lock-stores.mdx @@ -156,7 +156,7 @@ The `ConcurrentRedisLockStore` recreates the `TicketLock` from the persisted `Ti - `key_prefix` (default: `None`): The prefix to prepend to lock store keys. Must be alphanumeric - - `username` (default: `None`): Username used for authentication + - `username` (default: `None`): Username used for authentication - `password` (default: `None`): Password used for authentication (`None` equals no authentication) From 11660bb920b3cc74ef0968e70d1bb86de5c58d52 Mon Sep 17 00:00:00 2001 From: vcidst Date: Thu, 21 Sep 2023 11:30:00 +0200 Subject: [PATCH 05/11] change example_endpoints --- data/test_endpoints/example_endpoints.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/data/test_endpoints/example_endpoints.yml b/data/test_endpoints/example_endpoints.yml index fccc4f9c4594..705d9633a9cb 100644 --- a/data/test_endpoints/example_endpoints.yml +++ b/data/test_endpoints/example_endpoints.yml @@ -13,6 +13,7 @@ tracker_store: url: localhost port: 6379 db: 0 + username: username password: password key_prefix: conversation record_exp: 30000 From 8a85a6f7d4ebccec0ec3c8bfb6992610625de25f Mon Sep 17 00:00:00 2001 From: vcidst Date: Thu, 21 Sep 2023 12:52:47 +0200 Subject: [PATCH 06/11] update tests --- tests/core/test_lock_store.py | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/tests/core/test_lock_store.py b/tests/core/test_lock_store.py index e182e3beb5ef..bf2b1199b66e 100644 --- a/tests/core/test_lock_store.py +++ b/tests/core/test_lock_store.py @@ -1,35 +1,27 @@ import asyncio import logging import sys +import time from pathlib import Path +from typing import Text +from unittest.mock import Mock, patch import numpy as np import pytest -import time - +import rasa.core.lock_store from _pytest.logging import LogCaptureFixture from _pytest.monkeypatch import MonkeyPatch -from unittest.mock import patch, Mock - from rasa.core.agent import Agent from rasa.core.channels import UserMessage from rasa.core.constants import DEFAULT_LOCK_LIFETIME -from rasa.shared.constants import INTENT_MESSAGE_PREFIX from rasa.core.lock import TicketLock -import rasa.core.lock_store -from rasa.core.lock_store import ( - InMemoryLockStore, - LockError, - LockStore, - RedisLockStore, - DEFAULT_REDIS_LOCK_STORE_KEY_PREFIX, -) +from rasa.core.lock_store import (DEFAULT_REDIS_LOCK_STORE_KEY_PREFIX, + InMemoryLockStore, LockError, LockStore, + RedisLockStore) +from rasa.shared.constants import INTENT_MESSAGE_PREFIX from rasa.shared.exceptions import ConnectionException from rasa.utils.endpoints import EndpointConfig, read_endpoint_config -from typing import Text -from rasa.shared.core.domain import Domain - class FakeRedisLockStore(RedisLockStore): """Fake `RedisLockStore` using `fakeredis` library.""" @@ -389,8 +381,8 @@ async def test_redis_lock_store_with_valid_prefix(monkeypatch: MonkeyPatch): pass -def test_tracker_store_endpoint_config_loading(endpoints_path: Text): - cfg = read_endpoint_config(endpoints_path, "tracker_store") +def test_lock_store_endpoint_config_loading(endpoints_path: Text): + cfg = read_endpoint_config(endpoints_path, endpoint_type="lock_store") assert cfg == EndpointConfig.from_dict( { @@ -409,10 +401,9 @@ def test_tracker_store_endpoint_config_loading(endpoints_path: Text): ) -def test_create_lock_store_from_endpoint_config(domain: Domain, endpoints_path: Text): +def test_create_lock_store_from_endpoint_config(endpoints_path: Text): store = read_endpoint_config(endpoints_path, "tracker_store") tracker_store = RedisLockStore( - domain=domain, host="localhost", port=6379, db=0, @@ -425,4 +416,4 @@ def test_create_lock_store_from_endpoint_config(domain: Domain, endpoints_path: ssl_ca_certs="my-bundle.ca-bundle", ) - assert isinstance(tracker_store, type(LockStore.create(store, domain))) + assert isinstance(tracker_store, type(LockStore.create(store))) From 788f028a388a016e1cd3d203ed6962d68464815f Mon Sep 17 00:00:00 2001 From: vcidst Date: Thu, 21 Sep 2023 14:19:59 +0200 Subject: [PATCH 07/11] remove the duplicate test --- tests/core/test_lock_store.py | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/tests/core/test_lock_store.py b/tests/core/test_lock_store.py index bf2b1199b66e..368b88b3123f 100644 --- a/tests/core/test_lock_store.py +++ b/tests/core/test_lock_store.py @@ -15,9 +15,13 @@ from rasa.core.channels import UserMessage from rasa.core.constants import DEFAULT_LOCK_LIFETIME from rasa.core.lock import TicketLock -from rasa.core.lock_store import (DEFAULT_REDIS_LOCK_STORE_KEY_PREFIX, - InMemoryLockStore, LockError, LockStore, - RedisLockStore) +from rasa.core.lock_store import ( + DEFAULT_REDIS_LOCK_STORE_KEY_PREFIX, + InMemoryLockStore, + LockError, + LockStore, + RedisLockStore, +) from rasa.shared.constants import INTENT_MESSAGE_PREFIX from rasa.shared.exceptions import ConnectionException from rasa.utils.endpoints import EndpointConfig, read_endpoint_config @@ -381,26 +385,6 @@ async def test_redis_lock_store_with_valid_prefix(monkeypatch: MonkeyPatch): pass -def test_lock_store_endpoint_config_loading(endpoints_path: Text): - cfg = read_endpoint_config(endpoints_path, endpoint_type="lock_store") - - assert cfg == EndpointConfig.from_dict( - { - "type": "redis", - "url": "localhost", - "port": 6379, - "db": 0, - "username": "username", - "password": "password", - "timeout": 30000, - "use_ssl": True, - "ssl_keyfile": "keyfile.key", - "ssl_certfile": "certfile.crt", - "ssl_ca_certs": "my-bundle.ca-bundle", - } - ) - - def test_create_lock_store_from_endpoint_config(endpoints_path: Text): store = read_endpoint_config(endpoints_path, "tracker_store") tracker_store = RedisLockStore( @@ -409,7 +393,6 @@ def test_create_lock_store_from_endpoint_config(endpoints_path: Text): db=0, username="username", password="password", - record_exp=3000, use_ssl=True, ssl_keyfile="keyfile.key", ssl_certfile="certfile.crt", From 97c1f7d9e583f344481ecc09dfaa3506d3561ceb Mon Sep 17 00:00:00 2001 From: vcidst Date: Thu, 21 Sep 2023 16:22:05 +0200 Subject: [PATCH 08/11] add lock store config --- data/test_endpoints/example_endpoints.yml | 12 ++++++++++++ tests/core/test_lock_store.py | 1 + 2 files changed, 13 insertions(+) diff --git a/data/test_endpoints/example_endpoints.yml b/data/test_endpoints/example_endpoints.yml index 705d9633a9cb..75f397d6b0c7 100644 --- a/data/test_endpoints/example_endpoints.yml +++ b/data/test_endpoints/example_endpoints.yml @@ -21,6 +21,18 @@ tracker_store: ssl_keyfile: "keyfile.key" ssl_certfile: "certfile.crt" ssl_ca_certs: "my-bundle.ca-bundle" +lock_store: + type: redis + url: localhost + port: 6379 + db: 0 + username: username + password: password + key_prefix: lock + use_ssl: True + ssl_keyfile: "keyfile.key" + ssl_certfile: "certfile.crt" + ssl_ca_certs: "my-bundle.ca-bundle" # example of mongoDB external tracker store config #tracker_store: #type: mongod diff --git a/tests/core/test_lock_store.py b/tests/core/test_lock_store.py index 368b88b3123f..a73c5aa3d6aa 100644 --- a/tests/core/test_lock_store.py +++ b/tests/core/test_lock_store.py @@ -397,6 +397,7 @@ def test_create_lock_store_from_endpoint_config(endpoints_path: Text): ssl_keyfile="keyfile.key", ssl_certfile="certfile.crt", ssl_ca_certs="my-bundle.ca-bundle", + key_prefix="lock", ) assert isinstance(tracker_store, type(LockStore.create(store))) From a1690aa2b0ec0b977da5bb5e702056743c4e5fbe Mon Sep 17 00:00:00 2001 From: vcidst Date: Thu, 21 Sep 2023 16:43:34 +0200 Subject: [PATCH 09/11] change the endpoints yml file --- data/test_endpoints/lockstore_endpoints.yml | 33 +++++++++++++++++++++ tests/conftest.py | 3 ++ tests/core/test_lock_store.py | 4 +-- 3 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 data/test_endpoints/lockstore_endpoints.yml diff --git a/data/test_endpoints/lockstore_endpoints.yml b/data/test_endpoints/lockstore_endpoints.yml new file mode 100644 index 000000000000..38f840f6d4df --- /dev/null +++ b/data/test_endpoints/lockstore_endpoints.yml @@ -0,0 +1,33 @@ +nlg: + url: http://localhost:5055/nlg # url of the nlg endpoint + # you can also specify additional parameters, if you need them: + # headers: + # my-custom-header: value + # token: "my_authentication_token" # will be passed as a get parameter + # basic_auth: + # username: user + # password: pass +# example of redis external lock store config +lock_store: + type: redis + url: localhost + port: 6379 + db: 0 + username: username + password: password + key_prefix: lock + use_ssl: True + ssl_keyfile: "keyfile.key" + ssl_certfile: "certfile.crt" + ssl_ca_certs: "my-bundle.ca-bundle" +# example of mongoDB external tracker store config +#tracker_store: + #type: mongod + #url: mongodb://localhost:27017 + #db: rasa + #user: username + #password: password +action_endpoint: + url: http://localhost:5055/webhook + cafile: ./some_test_file +empty: diff --git a/tests/conftest.py b/tests/conftest.py index fdf0df3a1e1b..5e70cac7a45e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -204,6 +204,9 @@ def e2e_story_file_trips_circuit_breaker_path() -> Text: def endpoints_path() -> Text: return "data/test_endpoints/example_endpoints.yml" +@pytest.fixture(scope="session") +def lockstore_endpoints_path() -> Text: + return "data/test_endpoints/lockstore_endpoints.yml" # https://github.com/pytest-dev/pytest-asyncio/issues/68 # this event_loop is used by pytest-asyncio, and redefining it diff --git a/tests/core/test_lock_store.py b/tests/core/test_lock_store.py index a73c5aa3d6aa..6f99ee15de74 100644 --- a/tests/core/test_lock_store.py +++ b/tests/core/test_lock_store.py @@ -385,8 +385,8 @@ async def test_redis_lock_store_with_valid_prefix(monkeypatch: MonkeyPatch): pass -def test_create_lock_store_from_endpoint_config(endpoints_path: Text): - store = read_endpoint_config(endpoints_path, "tracker_store") +def test_create_lock_store_from_endpoint_config(lockstore_endpoints_path: Text): + store = read_endpoint_config(lockstore_endpoints_path, "tracker_store") tracker_store = RedisLockStore( host="localhost", port=6379, From 7e861ba78d64b5452862b832eff0a689aa81f1d1 Mon Sep 17 00:00:00 2001 From: vcidst Date: Thu, 21 Sep 2023 16:48:05 +0200 Subject: [PATCH 10/11] change the endpoint type --- data/test_endpoints/example_endpoints.yml | 1 + data/test_endpoints/lockstore_endpoints.yml | 33 --------------------- tests/conftest.py | 4 --- tests/core/test_lock_store.py | 4 +-- 4 files changed, 3 insertions(+), 39 deletions(-) delete mode 100644 data/test_endpoints/lockstore_endpoints.yml diff --git a/data/test_endpoints/example_endpoints.yml b/data/test_endpoints/example_endpoints.yml index 75f397d6b0c7..2e07554cee98 100644 --- a/data/test_endpoints/example_endpoints.yml +++ b/data/test_endpoints/example_endpoints.yml @@ -21,6 +21,7 @@ tracker_store: ssl_keyfile: "keyfile.key" ssl_certfile: "certfile.crt" ssl_ca_certs: "my-bundle.ca-bundle" +# example of redis external lock store config lock_store: type: redis url: localhost diff --git a/data/test_endpoints/lockstore_endpoints.yml b/data/test_endpoints/lockstore_endpoints.yml deleted file mode 100644 index 38f840f6d4df..000000000000 --- a/data/test_endpoints/lockstore_endpoints.yml +++ /dev/null @@ -1,33 +0,0 @@ -nlg: - url: http://localhost:5055/nlg # url of the nlg endpoint - # you can also specify additional parameters, if you need them: - # headers: - # my-custom-header: value - # token: "my_authentication_token" # will be passed as a get parameter - # basic_auth: - # username: user - # password: pass -# example of redis external lock store config -lock_store: - type: redis - url: localhost - port: 6379 - db: 0 - username: username - password: password - key_prefix: lock - use_ssl: True - ssl_keyfile: "keyfile.key" - ssl_certfile: "certfile.crt" - ssl_ca_certs: "my-bundle.ca-bundle" -# example of mongoDB external tracker store config -#tracker_store: - #type: mongod - #url: mongodb://localhost:27017 - #db: rasa - #user: username - #password: password -action_endpoint: - url: http://localhost:5055/webhook - cafile: ./some_test_file -empty: diff --git a/tests/conftest.py b/tests/conftest.py index 5e70cac7a45e..a9804cea978e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -204,10 +204,6 @@ def e2e_story_file_trips_circuit_breaker_path() -> Text: def endpoints_path() -> Text: return "data/test_endpoints/example_endpoints.yml" -@pytest.fixture(scope="session") -def lockstore_endpoints_path() -> Text: - return "data/test_endpoints/lockstore_endpoints.yml" - # https://github.com/pytest-dev/pytest-asyncio/issues/68 # this event_loop is used by pytest-asyncio, and redefining it # is currently the only way of changing the scope of this fixture diff --git a/tests/core/test_lock_store.py b/tests/core/test_lock_store.py index 6f99ee15de74..d90cd26a61b5 100644 --- a/tests/core/test_lock_store.py +++ b/tests/core/test_lock_store.py @@ -385,8 +385,8 @@ async def test_redis_lock_store_with_valid_prefix(monkeypatch: MonkeyPatch): pass -def test_create_lock_store_from_endpoint_config(lockstore_endpoints_path: Text): - store = read_endpoint_config(lockstore_endpoints_path, "tracker_store") +def test_create_lock_store_from_endpoint_config(endpoints_path: Text): + store = read_endpoint_config(endpoints_path, endpoint_type="lock_store") tracker_store = RedisLockStore( host="localhost", port=6379, From 8823da5c4c6f0bbfee62492444b874e282f38c22 Mon Sep 17 00:00:00 2001 From: vcidst Date: Thu, 21 Sep 2023 16:57:55 +0200 Subject: [PATCH 11/11] reformatted conftest --- tests/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/conftest.py b/tests/conftest.py index a9804cea978e..fdf0df3a1e1b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -204,6 +204,7 @@ def e2e_story_file_trips_circuit_breaker_path() -> Text: def endpoints_path() -> Text: return "data/test_endpoints/example_endpoints.yml" + # https://github.com/pytest-dev/pytest-asyncio/issues/68 # this event_loop is used by pytest-asyncio, and redefining it # is currently the only way of changing the scope of this fixture