Skip to content

fix: Add possibility to overwrite send_receive_timeout for clickhouse offline store #5792

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.

Sign up for GitHub

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

Merged
ntkathole merged 4 commits into from
Dec 23, 2025
Merged

fix: Add possibility to overwrite send_receive_timeout for clickhouse offline store #5792

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2f5a1c1
add changes + fix linting issues
astronautas Dec 22, 2025
24f6f40
generalize based on pr suggestion
astronautas Dec 22, 2025
a05c6ab
add test verifying config parsing
astronautas Dec 22, 2025
a3b9977
rework test into integration, since CH instance is needed
astronautas Dec 22, 2025
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
Prev Previous commit
rework test into integration, since CH instance is needed 
Signed-off-by: lukas.valatka <lukas.valatka@cast.ai>
  • Loading branch information
@astronautas @ntkathole
astronautas authored and ntkathole committed Dec 23, 2025
commit a3b9977448234ffc026963fbf7c1480a3fd28ab3
28 changes: 28 additions & 0 deletions sdk/python/feast/infra/offline_stores/contrib/clickhouse_offline_store/tests/data_source.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
from feast.infra.offline_stores.contrib.clickhouse_offline_store.clickhouse_source import (
ClickhouseSource,
)
from feast.infra.utils.clickhouse.clickhouse_config import ClickhouseConfig
from feast.infra.utils.clickhouse.connection_utils import get_client
from tests.integration.feature_repos.universal.data_source_creator import (
DataSourceCreator,
)
Expand Down Expand Up @@ -114,3 +116,29 @@ def create_saved_dataset_destination(self):

def teardown(self):
pass


def test_get_client_with_additional_params(clickhouse_container):
"""
Test that get_client works with a real ClickHouse container and properly passes
additional settings like send_receive_timeout.
"""
# Create config with custom send_receive_timeout
config = ClickhouseConfig(
host=clickhouse_container.get_container_host_ip(),
port=clickhouse_container.get_exposed_port(8123),
user=CLICKHOUSE_USER,
password=CLICKHOUSE_PASSWORD,
database=CLICKHOUSE_OFFLINE_DB,
additional_client_args={"send_receive_timeout": 60},
)

# Get client and verify it works
client = get_client(config)

# Verify client is connected and functional by running a simple query
result = client.query("SELECT 1 AS test_value")
assert result.result_rows == [(1,)]

# Verify the send_receive_timeout was applied
assert client.timeout._read == 60
51 changes: 0 additions & 51 deletions sdk/python/tests/unit/infra/offline_stores/test_clickhouse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
from unittest.mock import MagicMock, patch

import pytest
from testcontainers.clickhouse import ClickHouseContainer
from testcontainers.core.waiting_utils import wait_for_logs

from feast.infra.utils.clickhouse.clickhouse_config import ClickhouseConfig
from feast.infra.utils.clickhouse.connection_utils import get_client, thread_local
Expand Down Expand Up @@ -135,52 +133,3 @@ def test_clickhouse_config_handles_none_additional_client_args():
config = ClickhouseConfig(**raw_config)

assert config.additional_client_args is None


@pytest.fixture(scope="module")
def clickhouse_container():
"""Start a ClickHouse container for integration testing."""
container = ClickHouseContainer(
username="default",
password="password",
dbname="default",
)
container.start()

log_string_to_wait_for = "Logging errors to"
waited = wait_for_logs(
container=container,
predicate=log_string_to_wait_for,
timeout=30,
interval=10,
)
logger.info("Waited for %s seconds until ClickHouse container was up", waited)

yield container
container.stop()


def test_get_client_with_additional_params(clickhouse_container):
"""
Test that get_client works with a real ClickHouse container and properly passes
additional settings like send_receive_timeout.
"""
# Create config with custom send_receive_timeout
config = ClickhouseConfig(
host=clickhouse_container.get_container_host_ip(),
port=clickhouse_container.get_exposed_port(8123),
user="default",
password="password",
database="default",
additional_client_args={"send_receive_timeout": 60},
)

# Get client and verify it works
client = get_client(config)

# Verify client is connected and functional by running a simple query
result = client.query("SELECT 1 AS test_value")
assert result.result_rows == [(1,)]

# Verify the send_receive_timeout was applied
assert client.timeout._read == 60
Loading