Repository navigation
TPT-4130: Add Valkey Managed Database integration tests #932
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
base: dev
Are you sure you want to change the base?
Changes from all commits
fd7cc7d
c1b63e9
496f407
7612779
b84c15a
f914f8d
e3e1f4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,6 +71,18 @@ def handle_databases_postgres_config_view( | |
| return databases_postgres_config_view_output(json_data) | ||
|
|
||
|
|
||
| @output_override("databases", "valkey-config-view", OutputMode.table) | ||
| def handle_databases_valkey_config_view( | ||
| operation, output_handler, json_data | ||
| ) -> bool: | ||
| # pylint: disable=unused-argument | ||
| """ | ||
| Override the output of 'linode-cli databases valkey-config-view' | ||
| to properly display the valkey engine config. | ||
| """ | ||
| return databases_valkey_config_view_output(json_data) | ||
|
|
||
|
|
||
| @output_override("domains", "zone-file", OutputMode.delimited) | ||
| def handle_domains_zone_file(operation, output_handler, json_data) -> bool: | ||
| # pylint: disable=unused-argument | ||
|
|
@@ -317,11 +329,26 @@ def pg_view_output(json_data) -> bool: | |
| return False | ||
|
|
||
|
|
||
| def add_param_row(output, param_name, param_data): | ||
| def _format_param_type(param_type) -> str: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this is intentional, but it's worth noting in this context that the logic here makes the behavior for Valkey parameters with multiple types different from the other engines. I can't immediately produce an example for PostgreSQL, which is currently broken in the CLI (!), but I can for MySQL: ... where the type of the parameter would be given as Maybe this is a question best addressed to the maintainers, but: what's the right thing to do here? Aim for consistency across the engine types? If so, should MySQL + (presumably) PG switch to the formatting applied here, or should Valkey use the existing formatting? |
||
| """ | ||
| Format a DB Config parameter type, which may be a list for nullable | ||
| parameters (e.g. ["integer", "null"] -> "integer | null"). | ||
| """ | ||
| if isinstance(param_type, list): | ||
| return " | ".join(str(t) for t in param_type) | ||
|
|
||
| return str(param_type) | ||
|
|
||
|
|
||
| def add_param_row(output, param_name, param_data, format_nullable_type=False): | ||
| """ | ||
| Construct and add a row to the output table for DB Config view overrides. | ||
| """ | ||
| param_type = str(param_data.get("type", "")) | ||
| param_type = param_data.get("type", "") | ||
| if format_nullable_type: | ||
| param_type = _format_param_type(param_type) | ||
| else: | ||
| param_type = str(param_type) | ||
| example = str(param_data.get("example", "")) | ||
| minimum = str(param_data.get("minimum", "")) | ||
| maximum = str(param_data.get("maximum", "")) | ||
|
|
@@ -409,3 +436,29 @@ def databases_postgres_config_view_output(json_data) -> bool: | |
| console.print(output) | ||
|
|
||
| return False | ||
|
|
||
|
|
||
| def databases_valkey_config_view_output(json_data) -> bool: | ||
| """ | ||
| Parse and format the Valkey configuration output table. | ||
| """ | ||
| output = Table(header_style="bold", show_lines=True) | ||
|
|
||
| output.add_column("Parameter", style="bold") | ||
| output.add_column("Type", justify="center") | ||
| output.add_column("Example", justify="center") | ||
| output.add_column("Min", justify="center") | ||
| output.add_column("Max", justify="center") | ||
| output.add_column("Min Length", justify="center") | ||
| output.add_column("Max Length", justify="center") | ||
| output.add_column("Pattern", justify="center") | ||
| output.add_column("Requires Restart", justify="center") | ||
| output.add_column("Description", style="dim") | ||
|
|
||
| for field, params in json_data.items(): | ||
| add_param_row(output, field, params, format_nullable_type=True) | ||
|
|
||
| console = Console() | ||
| console.print(output) | ||
|
|
||
| return False | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,11 @@ | ||
| import json | ||
|
|
||
| import pytest | ||
|
|
||
| from tests.integration.database.helpers import wait_for_database_status | ||
| from tests.integration.helpers import ( | ||
| BASE_CMDS, | ||
| DEFAULT_REGION, | ||
| delete_target_id, | ||
| exec_test_command, | ||
| get_random_text, | ||
|
|
@@ -76,3 +80,96 @@ def mysql_cluster(): | |
| delete_target_id( | ||
| target="databases", delete_command="mysql-delete", id=database_id | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def valkey_cluster(): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The approach in
|
||
| commands = BASE_CMDS["databases"] | ||
| engines = json.loads(exec_test_command(commands + ["engines", "--json"])) | ||
| engine = next( | ||
| (item["id"] for item in engines if item["id"].startswith("valkey/")), | ||
| None, | ||
| ) | ||
| assert engine is not None, "No Valkey engine is available" | ||
|
|
||
| types = json.loads(exec_test_command(commands + ["types", "--json"])) | ||
| plans = [ | ||
| (item["id"], item["engines"]["price"]["hourly"]) | ||
| for item in types | ||
| if item.get("_split") == "valkey" and item["engines"]["quantity"] == 1 | ||
| ] | ||
| assert plans, "No single-node Valkey database type is available" | ||
| plan = min(plans, key=lambda entry: entry[1])[0] | ||
|
|
||
| regions = json.loads( | ||
| exec_test_command(["linode-cli", "regions", "list", "--json"]) | ||
| ) | ||
| supported_regions = [ | ||
| item["id"] | ||
| for item in regions | ||
| if "Managed Databases" in item["capabilities"] | ||
| and item.get("status") == "ok" | ||
| ] | ||
| region = ( | ||
| DEFAULT_REGION | ||
| if DEFAULT_REGION in supported_regions | ||
| else next(iter(supported_regions), None) | ||
| ) | ||
| assert region is not None, "No available Managed Databases region found" | ||
|
|
||
| label = get_random_text(8) + "-valkey" | ||
| created = json.loads( | ||
| exec_test_command( | ||
| commands | ||
| + [ | ||
| "valkey-create", | ||
| "--engine", | ||
| engine, | ||
| "--label", | ||
| label, | ||
| "--region", | ||
| region, | ||
| "--type", | ||
| plan, | ||
| "--engine_config.valkey_timeout", | ||
| "301", | ||
| "--engine_config.valkey_active_expire_effort", | ||
| "2", | ||
| "--engine_config.valkey_lfu_decay_time", | ||
| "2", | ||
| "--engine_config.valkey_lfu_log_factor", | ||
| "11", | ||
| "--engine_config.valkey_maxmemory_policy", | ||
| "allkeys-lru", | ||
| "--no-defaults", | ||
| "--json", | ||
| ] | ||
| ) | ||
| )[0] | ||
| database_id = created["id"] | ||
|
|
||
| try: | ||
| wait_for_database_status("valkey", database_id, "active") | ||
| yield { | ||
| "id": database_id, | ||
| "engine": engine, | ||
| "label": label, | ||
| "region": region, | ||
| "type": plan, | ||
| "engine_config": { | ||
| "valkey_timeout": 301, | ||
| "valkey_active_expire_effort": 2, | ||
| "valkey_lfu_decay_time": 2, | ||
| "valkey_lfu_log_factor": 11, | ||
| "valkey_maxmemory_policy": "allkeys-lru", | ||
| }, | ||
| } | ||
| finally: | ||
| delete_target_id( | ||
| target="databases", | ||
| delete_command="valkey-delete", | ||
| id=str(database_id), | ||
| use_retry=True, | ||
| retries=5, | ||
| delay=15, | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: not sure how consistently the convention is followed in this repo, but in this module at least it looks like the step-down rule more-or-less applies, which means this should follow
add_param_row().