From c6e40c9173fad011558418794fb56106deb1b5d8 Mon Sep 17 00:00:00 2001 From: DvirDukhan Date: Sun, 11 Jul 2021 23:24:48 +0300 Subject: [PATCH 1/3] Update redis.py The Redis connection string is a comma-separated string, where each additional parameter in addition to host and port is in the format `key=value`. This proposal fixing the parsing in cases where the parameter values contain the `=` symbol, for example `localhost:6379,password=my=password` Signed-off-by: DvirDukhan --- sdk/python/feast/infra/online_stores/redis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/feast/infra/online_stores/redis.py b/sdk/python/feast/infra/online_stores/redis.py index bb85a8e853..a226c5cd18 100644 --- a/sdk/python/feast/infra/online_stores/redis.py +++ b/sdk/python/feast/infra/online_stores/redis.py @@ -102,7 +102,7 @@ def _parse_connection_string(connection_string: str): params = {} for c in connection_string.split(","): if "=" in c: - kv = c.split("=") + kv = c.split("=", 1) try: kv[1] = json.loads(kv[1]) except json.JSONDecodeError: From 34e77dbbe853455f7a9b5db0af246b8e502ecd05 Mon Sep 17 00:00:00 2001 From: DvirDukhan Date: Tue, 13 Jul 2021 02:27:04 +0300 Subject: [PATCH 2/3] added test Signed-off-by: DvirDukhan --- sdk/python/tests/test_cli_redis.py | 46 +++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/sdk/python/tests/test_cli_redis.py b/sdk/python/tests/test_cli_redis.py index e948bffd25..cdc06021a4 100644 --- a/sdk/python/tests/test_cli_redis.py +++ b/sdk/python/tests/test_cli_redis.py @@ -3,7 +3,7 @@ import tempfile from pathlib import Path from textwrap import dedent - +import redis import pytest from feast.feature_store import FeatureStore @@ -58,3 +58,47 @@ def test_basic() -> None: result = runner.run(["teardown"], cwd=repo_path) assert result.returncode == 0 + + +@pytest.mark.integration +def test_connection_error() -> None: + project_id = "".join( + random.choice(string.ascii_lowercase + string.digits) for _ in range(10) + ) + runner = CliRunner() + with tempfile.TemporaryDirectory() as repo_dir_name, tempfile.TemporaryDirectory() as data_dir_name: + + repo_path = Path(repo_dir_name) + data_path = Path(data_dir_name) + + repo_config = repo_path / "feature_store.yaml" + + repo_config.write_text( + dedent( + f""" + project: {project_id} + registry: {data_path / "registry.db"} + provider: local + offline_store: + type: file + online_store: + type: redis + connection_string: localhost:6379,db=0= + """ + ) + ) + + repo_example = repo_path / "example.py" + repo_example.write_text( + (Path(__file__).parent / "example_feature_repo_2.py").read_text() + ) + + result = runner.run(["apply"], cwd=repo_path) + assert result.returncode == 0 + + # Redis does not support names for its databases. + with pytest.raises(redis.exceptions.ResponseError): + basic_rw_test( + FeatureStore(repo_path=str(repo_path), config=None), + view_name="driver_hourly_stats", + ) From dabb5494a669f52b69de8a3ef280e6be66c96582 Mon Sep 17 00:00:00 2001 From: DvirDukhan Date: Wed, 14 Jul 2021 00:00:33 +0300 Subject: [PATCH 3/3] format Signed-off-by: DvirDukhan --- sdk/python/tests/test_cli_redis.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sdk/python/tests/test_cli_redis.py b/sdk/python/tests/test_cli_redis.py index cdc06021a4..7aeafad83f 100644 --- a/sdk/python/tests/test_cli_redis.py +++ b/sdk/python/tests/test_cli_redis.py @@ -3,8 +3,9 @@ import tempfile from pathlib import Path from textwrap import dedent -import redis + import pytest +import redis from feast.feature_store import FeatureStore from tests.cli_utils import CliRunner @@ -95,7 +96,7 @@ def test_connection_error() -> None: result = runner.run(["apply"], cwd=repo_path) assert result.returncode == 0 - + # Redis does not support names for its databases. with pytest.raises(redis.exceptions.ResponseError): basic_rw_test(