Skip to content

Commit

Permalink
clickhouse readonly 1 option support added
Browse files Browse the repository at this point in the history
  • Loading branch information
juliarbkv committed Oct 16, 2024
1 parent 56ea7f0 commit fc17bb0
Show file tree
Hide file tree
Showing 26 changed files with 256 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ def get_request_params(self, dba_q: DBAdapterQuery) -> dict[str, str]:
database=dba_q.db_name or self._target_dto.db_name or "system",
**get_ch_settings(
read_only_level=2,
max_execution_time=self._target_dto.max_execution_time,
),
)

Expand Down
1 change: 0 additions & 1 deletion lib/dl_connector_chyt/dl_connector_chyt/core/adapters.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ def _get_dsn_params_from_headers(self) -> dict[str, str]:

def get_ch_settings(self) -> dict:
return get_ch_settings(
max_execution_time=self._target_dto.max_execution_time,
read_only_level=2,
output_format_json_quote_denormals=1,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,4 @@ class ClickHouseConnectionSchema(
load_default=None,
load_only=True,
)
readonly = ma_fields.Integer(attribute="data.readonly", bi_extra=FieldExtra(editable=True))
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
from enum import unique
import typing

import attr

from dl_api_commons.base_models import TenantDef
from dl_api_connector.form_config.models import rows as C
from dl_api_connector.form_config.models.api_schema import (
FormActionApiSchema,
FormApiSchema,
Expand All @@ -12,16 +16,52 @@
ConnectionFormFactory,
ConnectionFormMode,
)
from dl_api_connector.form_config.models.common import CommonFieldName
import dl_api_connector.form_config.models.rows as api_rows
from dl_api_connector.form_config.models.common import (
CommonFieldName,
FormFieldName,
)
from dl_api_connector.form_config.models.rows.base import FormRow
from dl_api_connector.form_config.models.shortcuts.rows import RowConstructor
from dl_configs.connectors_settings import ConnectorSettingsBase
from dl_i18n.localizer_base import Localizer

from dl_connector_clickhouse.api.connection_info import ClickHouseConnectionInfoProvider
from dl_connector_clickhouse.api.i18n.localizer import Translatable


@unique
class ClickHouseFieldName(FormFieldName):
readonly = "readonly"


@attr.s
class ClickHouseRowConstructor:
_localizer: Localizer = attr.ib()

def readonly_mode_row(self) -> C.CustomizableRow:
return C.CustomizableRow(
items=[
C.LabelRowItem(
align="start",
text=self._localizer.translate(Translatable("field_readonly-mode")),
display_conditions={CommonFieldName.advanced_settings: "opened"},
),
C.SelectRowItem(
name=ClickHouseFieldName.readonly,
width="s",
available_values=[
C.SelectOption(content="0", value="0"),
C.SelectOption(content="1", value="1"),
C.SelectOption(content="2", value="2"),
],
default_value="2",
control_props=C.SelectRowItem.Props(show_search=False),
display_conditions={CommonFieldName.advanced_settings: "opened"},
),
]
)


class ClickHouseConnectionFormFactory(ConnectionFormFactory):
DEFAULT_PORT = "8443"

Expand All @@ -38,6 +78,7 @@ def get_common_api_schema_items() -> list[FormFieldApiSchema]:
FormFieldApiSchema(name=CommonFieldName.secure),
FormFieldApiSchema(name=CommonFieldName.ssl_ca),
FormFieldApiSchema(name=CommonFieldName.data_export_forbidden),
FormFieldApiSchema(name=ClickHouseFieldName.readonly),
]

def _get_edit_api_schema(
Expand Down Expand Up @@ -109,8 +150,9 @@ def _get_common_section(
rc: RowConstructor,
connector_settings: ConnectorSettingsBase | None,
) -> typing.Sequence[FormRow]:
clickhouse_rc = ClickHouseRowConstructor(localizer=self._localizer)
return [
api_rows.CacheTTLRow(name=CommonFieldName.cache_ttl_sec),
C.CacheTTLRow(name=CommonFieldName.cache_ttl_sec),
rc.raw_sql_level_row(),
rc.collapse_advanced_settings_row(),
*rc.ssl_rows(
Expand All @@ -119,6 +161,7 @@ def _get_common_section(
enabled_default_value=True,
),
rc.data_export_forbidden_row(),
clickhouse_rc.readonly_mode_row(),
]

def get_form_config(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ def get_conn_line(self, db_name: Optional[str] = None, params: Optional[dict[str
# TODO FIX: Move to utils
def get_ch_settings(self) -> dict:
return get_ch_settings(
max_execution_time=self._target_dto.max_execution_time,
read_only_level=None,
output_format_json_quote_denormals=1,
)
Expand Down Expand Up @@ -401,13 +400,17 @@ def get_session_headers(self) -> dict[str, str]:
}

def get_request_params(self, dba_q: DBAdapterQuery) -> dict[str, str]:
read_only_level = None if dba_q.trusted_query else 2
if dba_q.trusted_query:
read_only_level = None
elif self._target_dto.readonly == 1:
read_only_level = 1
else:
read_only_level = 2
return dict(
# TODO FIX: Move to utils
database=dba_q.db_name or self._target_dto.db_name or "system",
**get_ch_settings(
read_only_level=read_only_level,
max_execution_time=self._target_dto.max_execution_time,
),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,13 @@

def get_ch_settings(
read_only_level: Optional[int] = None,
max_execution_time: Optional[int] = None,
output_format_json_quote_denormals: Optional[int] = None,
) -> dict:
settings = {
# https://clickhouse.com/docs/en/operations/settings/settings#settings-join_use_nulls
# 1 — JOIN behaves the same way as in standard SQL.
# The type of the corresponding field is converted to Nullable, and empty cells are filled with NULL.
"join_use_nulls": 1,
# https://clickhouse.com/docs/en/operations/settings/query-complexity#max-execution-time
# Maximum query execution time in seconds.
# By default, specify a large value to ensure there are no
# forever-running queries (which is also known to break old-version CH
# hosts at around 100_000 second long queries).
# Note that in CH the value is rounded down to integer, and 0 seems to mean 'no limit'.
# TODO: get rid of this parameter or figure out a proper way to use it, bc CH's estimates seem to be way off
"max_execution_time": max_execution_time if max_execution_time is None else 3600 * 4,
"readonly": read_only_level,
# request clickhouse stat in response headers
# otherwise clickhouse sends nulls in X-ClickHouse-Summary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ async def _make_target_conn_dto_pool(self) -> list[ClickHouseConnTargetDTO]: #
secure=self._conn_dto.secure,
ssl_ca=self._conn_dto.ssl_ca,
ca_data=self._ca_data.decode("ascii"),
readonly=self._conn_dto.readonly,
)
)
return dto_pool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ class ClickHouseConnDTO(ClickHouseBaseDTO, DefaultSQLDTO): # noqa
# TODO CONSIDER: Is really optional
endpoint: Optional[str] = attr.ib(kw_only=True)
cluster_name: str = attr.ib(kw_only=True)
readonly: int = attr.ib(kw_only=True)
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ class CHIncorrectData(CHQueryError):

class CHReadonlyUser(CHQueryError):
err_code = CHQueryError.err_code + ["READONLY_USER"]
default_message = "Clickhouse user should have parameter readonly set to 0 or 2"
default_message = (
"Clickhouse user must be correctly configured to use readonly 1 option (see docs). "
"For other readonly options user should have parameter readonly set to 0 or 2."
)


class EstimatedExecutionTooLong(exc.DatabaseQueryError):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ class ConnectionClickHouseBaseDataStorageSchema(
endpoint = ma_fields.String(required=False, allow_none=True, load_default=None, dump_default=None)
cluster_name = ma_fields.String(required=False, allow_none=True, load_default=None, dump_default=None)
max_execution_time = ma_fields.Integer(required=False, allow_none=True, load_default=None, dump_default=None)
readonly = ma_fields.Integer(required=False, allow_none=True, load_default=None, dump_default=None)
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class BaseClickHouseConnTargetDTO(BaseSQLConnTargetDTO, BaseAiohttpConnTargetDTO
disable_value_processing: bool = attr.ib()
secure: bool = attr.ib(kw_only=True, default=False)
ssl_ca: Optional[str] = attr.ib(kw_only=True, default=None)
readonly: int = attr.ib(kw_only=True, default=None)

# TODO remove in the next release to avoid compatibility issues
insert_quorum: Optional[int] = attr.ib(kw_only=True, default=None)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class DataModel(ClassicConnectionSQL.DataModel):
cluster_name: Optional[str] = attr.ib(default=None)
max_execution_time: Optional[int] = attr.ib(default=None)
ssl_ca: Optional[str] = attr.ib(kw_only=True, default=None)
readonly: int = attr.ib(kw_only=True, default=None)

def get_conn_dto(self) -> ClickHouseConnDTO:
return ClickHouseConnDTO(
Expand All @@ -69,6 +70,7 @@ def get_conn_dto(self) -> ClickHouseConnDTO:
password=self.password, # type: ignore # 2024-01-24 # TODO: Argument "password" to "ClickHouseConnDTO" has incompatible type "str | None"; expected "str" [arg-type]
secure=self.data.secure,
ssl_ca=self.data.ssl_ca,
readonly=self.data.readonly,
)

@staticmethod
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ msgstr ""
msgid "label_connector-clickhouse"
msgstr "ClickHouse"

msgid "field_readonly-mode"
msgstr "Readonly"

msgid "field_click-house-port"
msgstr "HTTP interface port"

Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ msgstr ""
msgid "label_connector-clickhouse"
msgstr "ClickHouse"

msgid "field_readonly-mode"
msgstr "Readonly"

msgid "field_click-house-port"
msgstr "Порт HTTP-интерфейса"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from dl_connector_clickhouse_tests.db.config import (
API_TEST_CONFIG,
CoreConnectionSettings,
CoreReadonlyConnectionSettings,
)
from dl_connector_clickhouse_tests.db.core.base import BaseClickHouseTestClass

Expand Down Expand Up @@ -45,6 +46,19 @@ def connection_params(self) -> dict:
)


class ClickHouseConnectionReadonlyUserTestBase(ClickHouseConnectionTestBase):
@pytest.fixture(scope="class")
def connection_params(self) -> dict:
return dict(
db_name=CoreReadonlyConnectionSettings.DB_NAME,
host=CoreReadonlyConnectionSettings.HOST,
port=CoreReadonlyConnectionSettings.PORT,
username=CoreReadonlyConnectionSettings.USERNAME,
password=CoreReadonlyConnectionSettings.PASSWORD,
readonly="1",
)


class ClickHouseDashSQLConnectionTest(ClickHouseConnectionTestBase):
raw_sql_level = RawSQLLevel.dashsql

Expand All @@ -61,5 +75,13 @@ def dataset_params(self, sample_table) -> dict:
)


class ClickHouseDatasetReadonlyUserTestBase(ClickHouseConnectionReadonlyUserTestBase, ClickHouseDatasetTestBase):
pass


class ClickHouseDataApiTestBase(ClickHouseDatasetTestBase, StandardizedDataApiTestBase):
mutation_caches_enabled = False


class ClickHouseDataApiReadonlyUserTestBase(ClickHouseDatasetReadonlyUserTestBase, StandardizedDataApiTestBase):
mutation_caches_enabled = False
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from dl_connector_clickhouse_tests.db.api.base import (
ClickHouseConnectionDefaultUserTestBase,
ClickHouseConnectionReadonlyUserTestBase,
ClickHouseConnectionTestBase,
)

Expand All @@ -12,3 +13,9 @@ class TestClickHouseConnection(ClickHouseConnectionTestBase, DefaultConnectorCon

class TestClickHouseDefaultUserConnection(ClickHouseConnectionDefaultUserTestBase, DefaultConnectorConnectionTestSuite):
pass


class TestClickHouseReadonlyUserConnection(
ClickHouseConnectionReadonlyUserTestBase, DefaultConnectorConnectionTestSuite
):
pass
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
DefaultConnectorDataResultTestSuite,
)

from dl_connector_clickhouse_tests.db.api.base import ClickHouseDataApiTestBase
from dl_connector_clickhouse_tests.db.api.base import (
ClickHouseDataApiReadonlyUserTestBase,
ClickHouseDataApiTestBase,
)


class TestClickHouseDataResult(ClickHouseDataApiTestBase, DefaultConnectorDataResultTestSuite):
Expand All @@ -32,3 +35,7 @@ class TestClickHouseDataPreview(ClickHouseDataApiTestBase, DefaultConnectorDataP

class TestClickHouseDataCache(ClickHouseDataApiTestBase, DefaultConnectorDataCacheTestSuite):
data_caches_enabled = True


class TestClickHouseReadonlyUserDataResult(ClickHouseDataApiReadonlyUserTestBase, DefaultConnectorDataResultTestSuite):
pass
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
from dl_api_lib_testing.connector.dataset_suite import DefaultConnectorDatasetTestSuite

from dl_connector_clickhouse_tests.db.api.base import ClickHouseDatasetTestBase
from dl_connector_clickhouse_tests.db.api.base import (
ClickHouseDatasetReadonlyUserTestBase,
ClickHouseDatasetTestBase,
)


class TestClickHouseDataset(ClickHouseDatasetTestBase, DefaultConnectorDatasetTestSuite):
pass


class TestClickHouseDatasetReadonlyUser(ClickHouseDatasetReadonlyUserTestBase, DefaultConnectorDatasetTestSuite):
pass
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ class CoreSslConnectionSettings:
PASSWORD: ClassVar[str] = "qwerty"


class CoreReadonlyConnectionSettings:
DB_NAME: ClassVar[str] = "test_data"
HOST: ClassVar[str] = get_test_container_hostport("db-clickhouse-22-10", fallback_port=52204).host
PORT: ClassVar[int] = get_test_container_hostport("db-clickhouse-22-10", fallback_port=52204).port
USERNAME: ClassVar[str] = "readonly"
PASSWORD: ClassVar[str] = "qwerty"


DASHSQL_QUERY = r"""select
arrayJoin([11, 22, NULL]) as a,
[33, 44] as b,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,19 @@ def connection_creation_params(self) -> dict:
)


class BaseClickHouseReadonlyUserTestClass(BaseClickHouseTestClass):
@pytest.fixture(scope="function")
def connection_creation_params(self) -> dict:
return dict(
db_name=test_config.CoreReadonlyConnectionSettings.DB_NAME,
host=test_config.CoreReadonlyConnectionSettings.HOST,
port=test_config.CoreReadonlyConnectionSettings.PORT,
username=test_config.CoreReadonlyConnectionSettings.USERNAME,
password=test_config.CoreReadonlyConnectionSettings.PASSWORD,
readonly="1",
)


class BaseSslClickHouseTestClass(BaseClickHouseTestClass):
@pytest.fixture(autouse=True)
def clear_ssl_folder(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from dl_connector_clickhouse.core.clickhouse.us_connection import ConnectionClickhouse
from dl_connector_clickhouse_tests.db.core.base import (
BaseClickHouseDefaultUserTestClass,
BaseClickHouseReadonlyUserTestClass,
BaseClickHouseTestClass,
BaseSslClickHouseTestClass,
)
Expand Down Expand Up @@ -52,6 +53,16 @@ def check_saved_connection(self, conn: ConnectionClickhouse, params: dict) -> No
assert conn.data.ssl_ca is None


class TestClickHouseReadonlyUserConnection(BaseClickHouseReadonlyUserTestClass, TestClickHouseConnection):
def check_saved_connection(self, conn: ConnectionClickhouse, params: dict) -> None:
assert conn.uuid is not None
assert conn.data.db_name == params["db_name"]
assert conn.data.username == params["username"]
assert conn.data.secure is False
assert conn.data.ssl_ca is None
assert conn.data.readonly == "1"


@pytest.mark.skipif(os.environ.get("WE_ARE_IN_CI"), reason="can't use localhost")
class TestSslClickHouseConnection(
BaseSslClickHouseTestClass,
Expand Down
Loading

0 comments on commit fc17bb0

Please sign in to comment.