From af8ce8817e0fba3dd94f6488d8b9d868544df441 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 12 Aug 2025 06:38:22 -0400 Subject: [PATCH 01/37] Rename generate-user-env-vars-file since it will also need to generate the worker's catalog. --- .../{generate-user-env-vars-file.py => generate-configs.py} | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) rename tools/deployment/presto-clp/scripts/{generate-user-env-vars-file.py => generate-configs.py} (98%) diff --git a/tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py b/tools/deployment/presto-clp/scripts/generate-configs.py similarity index 98% rename from tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py rename to tools/deployment/presto-clp/scripts/generate-configs.py index 470b09c048..57888996da 100644 --- a/tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py +++ b/tools/deployment/presto-clp/scripts/generate-configs.py @@ -28,7 +28,9 @@ def main(argv=None) -> int: argv = sys.argv args_parser = argparse.ArgumentParser( - description="Generates an environment variables file for any user-configured properties." + description=( + "Generates any necessary config files corresponding to user-configured properties." + ) ) args_parser.add_argument( "--clp-package-dir", help="CLP package directory.", required=True, type=Path From e0a1820e2b97705987c00800d8ba0be53782c987 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 12 Aug 2025 21:53:44 -0400 Subject: [PATCH 02/37] Move CLP config load out of _add_clp_env_vars. --- .../presto-clp/scripts/generate-configs.py | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/generate-configs.py b/tools/deployment/presto-clp/scripts/generate-configs.py index 57888996da..1912883ef0 100644 --- a/tools/deployment/presto-clp/scripts/generate-configs.py +++ b/tools/deployment/presto-clp/scripts/generate-configs.py @@ -2,7 +2,7 @@ import logging import sys from pathlib import Path -from typing import Dict, Optional +from typing import Any, Dict, Optional import yaml from dotenv import dotenv_values @@ -43,8 +43,20 @@ def main(argv=None) -> int: clp_package_dir: Path = parsed_args.clp_package_dir.resolve() output_file: Path = parsed_args.output_file + clp_config_file_path = clp_package_dir / "etc" / "clp-config.yml" + if not clp_config_file_path.exists(): + logger.error( + "'%s' doesn't exist. Is '%s' the location of the CLP package?", + clp_config_file_path, + clp_package_dir.resolve(), + ) + return False + + with open(clp_config_file_path, "r") as clp_config_file: + clp_config = yaml.safe_load(clp_config_file) + env_vars: Dict[str, str] = {} - if not _add_clp_env_vars(clp_package_dir, env_vars): + if not _add_clp_env_vars(clp_config, clp_package_dir, env_vars): return 1 script_dir = Path(__file__).parent.resolve() @@ -58,28 +70,19 @@ def main(argv=None) -> int: return 0 -def _add_clp_env_vars(clp_package_dir: Path, env_vars: Dict[str, str]) -> bool: +def _add_clp_env_vars( + clp_config: Dict[str, Any], clp_package_dir: Path, env_vars: Dict[str, str] +) -> bool: """ Adds environment variables for CLP config values to `env_vars`. + :param clp_config: :param clp_package_dir: :param env_vars: :return: Whether the environment variables were successfully added. """ env_vars["PRESTO_COORDINATOR_CLPPROPERTIES_METADATA_TABLE_PREFIX"] = "clp_" - clp_config_file_path = clp_package_dir / "etc" / "clp-config.yml" - if not clp_config_file_path.exists(): - logger.error( - "'%s' doesn't exist. Is '%s' the location of the CLP package?", - clp_config_file_path, - clp_package_dir.resolve(), - ) - return False - - with open(clp_config_file_path, "r") as clp_config_file: - clp_config = yaml.safe_load(clp_config_file) - database_type = _get_config_value(clp_config, "database.type", "mariadb") if "mariadb" != database_type and "mysql" != database_type: logger.error( From 4deb2bae022db8cd83663f6da73455e30a782186 Mon Sep 17 00:00:00 2001 From: anlowee Date: Mon, 18 Aug 2025 14:57:32 +0000 Subject: [PATCH 03/37] Add S3 support --- .../presto-clp/scripts/generate-configs.py | 20 +++++++++++++++++++ .../worker/scripts/generate-configs.sh | 11 ++++++++++ 2 files changed, 31 insertions(+) diff --git a/tools/deployment/presto-clp/scripts/generate-configs.py b/tools/deployment/presto-clp/scripts/generate-configs.py index 1912883ef0..f6c780cf72 100644 --- a/tools/deployment/presto-clp/scripts/generate-configs.py +++ b/tools/deployment/presto-clp/scripts/generate-configs.py @@ -139,6 +139,26 @@ def _add_clp_env_vars( env_vars["PRESTO_COORDINATOR_CLPPROPERTIES_METADATA_DATABASE_USER"] = database_user env_vars["PRESTO_COORDINATOR_CLPPROPERTIES_METADATA_DATABASE_PASSWORD"] = database_password + archive_out_storage_key_prefix = "archive_output.storage" + archive_out_storage_type = _get_config_value(clp_config, f"{archive_out_storage_key_prefix}.type") + if "s3" == archive_out_storage_type: + s3_config_key_prefix = f"{archive_out_storage_key_prefix}.s3_config" + s3_credentials_key_prefix = f"{s3_config_key_prefix}.aws_authentication.credentials" + + s3_access_key_id = _get_config_value(clp_config, f"{s3_credentials_key_prefix}.access_key_id") + + s3_bucket = _get_config_value(clp_config, f"{s3_config_key_prefix}.bucket") + s3_region_code = _get_config_value(clp_config, f"{s3_config_key_prefix}.region_code") + s3_end_point = f"https://{s3_bucket}.s3.{s3_region_code}.amazonaws.com/" + + s3_secret_access_key = _get_config_value(clp_config, f"{s3_credentials_key_prefix}.secret_access_key") + + env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"] = "s3" + env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER"] = "clp_package" + env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID"] = s3_access_key_id + env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT"] = s3_end_point + env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY"] = s3_secret_access_key + return True diff --git a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh index c04ae86182..f60a67f6ba 100755 --- a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh +++ b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh @@ -61,6 +61,17 @@ apt-get update && apt-get install --assume-yes --no-install-recommends jq wget readonly PRESTO_CONFIG_DIR="/opt/presto-server/etc" +# Update clp.properties to insert the environment variable templates +readonly CLP_PROPERTIES_FILE="/opt/presto-server/etc/catalog/clp.properties" +if [ -n "${PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER:-}" ]; then + log "INFO" "Enable S3 support" + update_config_file "$CLP_PROPERTIES_FILE" "clp.storage-type" "\${PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE}" + update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-auth-provider" "\${PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER}" + update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-access-key-id" "\${PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID}" + update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-end-point" "\${PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT}" + update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-secret-access-key" "\{PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY}" +fi + # Substitute environment variables in config template find /configs -type f | while read -r f; do ( From 728efe2b2bab1a08e37d7c385a275b7a283efec4 Mon Sep 17 00:00:00 2001 From: anlowee Date: Mon, 18 Aug 2025 15:17:14 +0000 Subject: [PATCH 04/37] Fix --- .../presto-clp/scripts/generate-configs.py | 74 ++++++++++--------- .../presto-clp/scripts/set-up-config.sh | 2 +- 2 files changed, 42 insertions(+), 34 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/generate-configs.py b/tools/deployment/presto-clp/scripts/generate-configs.py index f6c780cf72..9fd4317b77 100644 --- a/tools/deployment/presto-clp/scripts/generate-configs.py +++ b/tools/deployment/presto-clp/scripts/generate-configs.py @@ -103,24 +103,52 @@ def _add_clp_env_vars( clp_archive_output_storage_type = _get_config_value( clp_config, "archive_output.storage.type", "fs" ) - if "fs" != clp_archive_output_storage_type: + if "fs" == clp_archive_output_storage_type: + clp_archives_dir = _get_config_value( + clp_config, + "archive_output.storage.directory", + str(clp_package_dir / "var" / "data" / "archives"), + ) + if Path(clp_archives_dir).is_absolute(): + env_vars["CLP_ARCHIVES_DIR"] = clp_archives_dir + else: + env_vars["CLP_ARCHIVES_DIR"] = str(clp_package_dir / clp_archives_dir) + elif "s3" == clp_archive_output_storage_type: + # This will not be used, just to ensure CLP_ARCHIVES_DIR is not an empty string + clp_archives_dir = _get_config_value( + clp_config, + "archive_output.storage.directory", + str(clp_package_dir / "var" / "data" / "staged-archives"), + ) + if Path(clp_archives_dir).is_absolute(): + env_vars["CLP_ARCHIVES_DIR"] = clp_archives_dir + else: + env_vars["CLP_ARCHIVES_DIR"] = str(clp_package_dir / clp_archives_dir) + + s3_config_key_prefix = f"archive_output.storage.s3_config" + s3_credentials_key_prefix = f"{s3_config_key_prefix}.aws_authentication.credentials" + + s3_access_key_id = _get_config_value(clp_config, f"{s3_credentials_key_prefix}.access_key_id") + + s3_bucket = _get_config_value(clp_config, f"{s3_config_key_prefix}.bucket") + s3_region_code = _get_config_value(clp_config, f"{s3_config_key_prefix}.region_code") + s3_end_point = f"https://{s3_bucket}.s3.{s3_region_code}.amazonaws.com/" + + s3_secret_access_key = _get_config_value(clp_config, f"{s3_credentials_key_prefix}.secret_access_key") + + env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"] = "s3" + env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER"] = "clp_package" + env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID"] = s3_access_key_id + env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT"] = s3_end_point + env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY"] = s3_secret_access_key + else: logger.error( - "Expected CLP's archive_output.storage.type to be fs but found '%s'. Presto" - " currently only supports reading archives from the fs storage type.", + "Expected CLP's archive_output.storage.type to be fs or s3 but found '%s'. Presto" + " currently only supports reading archives from the fs or s3 storage type.", clp_archive_output_storage_type, ) return False - clp_archives_dir = _get_config_value( - clp_config, - "archive_output.storage.directory", - str(clp_package_dir / "var" / "data" / "archives"), - ) - if Path(clp_archives_dir).is_absolute(): - env_vars["CLP_ARCHIVES_DIR"] = clp_archives_dir - else: - env_vars["CLP_ARCHIVES_DIR"] = str(clp_package_dir / clp_archives_dir) - credentials_file_path = clp_package_dir / "etc" / "credentials.yml" if not credentials_file_path.exists(): logger.error("'%s' doesn't exist. Did you start CLP?", credentials_file_path) @@ -139,26 +167,6 @@ def _add_clp_env_vars( env_vars["PRESTO_COORDINATOR_CLPPROPERTIES_METADATA_DATABASE_USER"] = database_user env_vars["PRESTO_COORDINATOR_CLPPROPERTIES_METADATA_DATABASE_PASSWORD"] = database_password - archive_out_storage_key_prefix = "archive_output.storage" - archive_out_storage_type = _get_config_value(clp_config, f"{archive_out_storage_key_prefix}.type") - if "s3" == archive_out_storage_type: - s3_config_key_prefix = f"{archive_out_storage_key_prefix}.s3_config" - s3_credentials_key_prefix = f"{s3_config_key_prefix}.aws_authentication.credentials" - - s3_access_key_id = _get_config_value(clp_config, f"{s3_credentials_key_prefix}.access_key_id") - - s3_bucket = _get_config_value(clp_config, f"{s3_config_key_prefix}.bucket") - s3_region_code = _get_config_value(clp_config, f"{s3_config_key_prefix}.region_code") - s3_end_point = f"https://{s3_bucket}.s3.{s3_region_code}.amazonaws.com/" - - s3_secret_access_key = _get_config_value(clp_config, f"{s3_credentials_key_prefix}.secret_access_key") - - env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"] = "s3" - env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER"] = "clp_package" - env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID"] = s3_access_key_id - env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT"] = s3_end_point - env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY"] = s3_secret_access_key - return True diff --git a/tools/deployment/presto-clp/scripts/set-up-config.sh b/tools/deployment/presto-clp/scripts/set-up-config.sh index 6c6fb8cced..0b9a8cc546 100755 --- a/tools/deployment/presto-clp/scripts/set-up-config.sh +++ b/tools/deployment/presto-clp/scripts/set-up-config.sh @@ -23,6 +23,6 @@ echo "Installing required Python packages..." pip3 install -r "${script_dir}/requirements.txt" echo "Generating environment variables file for user-configured properties..." -python3 "${script_dir}/generate-user-env-vars-file.py" \ +python3 "${script_dir}/generate-configs.py" \ --clp-package-dir "${clp_package_dir}" \ --output-file "${script_dir}/../.env" From 2ba53ca0cb7cc0f9c4a3216b640a4a806a6b6749 Mon Sep 17 00:00:00 2001 From: anlowee Date: Mon, 18 Aug 2025 15:51:58 +0000 Subject: [PATCH 05/37] Fix --- .../worker/scripts/generate-configs.sh | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh index f60a67f6ba..8531f55e64 100755 --- a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh +++ b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh @@ -61,17 +61,6 @@ apt-get update && apt-get install --assume-yes --no-install-recommends jq wget readonly PRESTO_CONFIG_DIR="/opt/presto-server/etc" -# Update clp.properties to insert the environment variable templates -readonly CLP_PROPERTIES_FILE="/opt/presto-server/etc/catalog/clp.properties" -if [ -n "${PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER:-}" ]; then - log "INFO" "Enable S3 support" - update_config_file "$CLP_PROPERTIES_FILE" "clp.storage-type" "\${PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE}" - update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-auth-provider" "\${PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER}" - update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-access-key-id" "\${PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID}" - update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-end-point" "\${PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT}" - update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-secret-access-key" "\{PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY}" -fi - # Substitute environment variables in config template find /configs -type f | while read -r f; do ( @@ -85,6 +74,17 @@ done rm -f "${PRESTO_CONFIG_DIR}/catalog/"* mv "${PRESTO_CONFIG_DIR}/clp.properties" "${PRESTO_CONFIG_DIR}/catalog" +# Update clp.properties +readonly CLP_PROPERTIES_FILE="/opt/presto-server/etc/catalog/clp.properties" +if [ -n "${PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER:-}" ]; then + log "INFO" "Enable S3 support" + update_config_file "$CLP_PROPERTIES_FILE" "clp.storage-type" "${PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE}" + update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-auth-provider" "${PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER}" + update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-access-key-id" "${PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID}" + update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-end-point" "${PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT}" + update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-secret-access-key" "${PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY}" +fi + # Update config.properties readonly CONFIG_PROPERTIES_FILE="/opt/presto-server/etc/config.properties" version=$(get_coordinator_version "$CONFIG_PROPERTIES_FILE") From aa3e18d8b9c07acd2d5d536d65f10b8bf0e56cfd Mon Sep 17 00:00:00 2001 From: anlowee Date: Mon, 18 Aug 2025 19:19:20 +0000 Subject: [PATCH 06/37] Add logs --- components/core/src/clp_s/InputConfig.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/core/src/clp_s/InputConfig.cpp b/components/core/src/clp_s/InputConfig.cpp index 61fe5415a2..9bcb0ace13 100644 --- a/components/core/src/clp_s/InputConfig.cpp +++ b/components/core/src/clp_s/InputConfig.cpp @@ -16,6 +16,8 @@ #include "../clp/spdlog_with_specializations.hpp" #include "Utils.hpp" +#include + namespace clp_s { auto get_source_for_path(std::string_view const path) -> InputSource { try { @@ -146,9 +148,11 @@ auto try_create_network_reader(std::string_view const url, NetworkAuthOption con std::string request_url{url}; switch (auth.method) { case AuthMethod::S3PresignedUrlV4: + std::cout << "XWEI before sign: " << request_url << std::endl; if (false == try_sign_url(request_url)) { return nullptr; } + std::cout << "XWEI after sign: " << request_url << std::endl; break; case AuthMethod::None: break; From 67e16ce9eae4a2cabe92cbcbb106b979ae147707 Mon Sep 17 00:00:00 2001 From: anlowee Date: Mon, 18 Aug 2025 19:25:02 +0000 Subject: [PATCH 07/37] Revert "Add logs" This reverts commit aa3e18d8b9c07acd2d5d536d65f10b8bf0e56cfd. --- components/core/src/clp_s/InputConfig.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/components/core/src/clp_s/InputConfig.cpp b/components/core/src/clp_s/InputConfig.cpp index 9bcb0ace13..61fe5415a2 100644 --- a/components/core/src/clp_s/InputConfig.cpp +++ b/components/core/src/clp_s/InputConfig.cpp @@ -16,8 +16,6 @@ #include "../clp/spdlog_with_specializations.hpp" #include "Utils.hpp" -#include - namespace clp_s { auto get_source_for_path(std::string_view const path) -> InputSource { try { @@ -148,11 +146,9 @@ auto try_create_network_reader(std::string_view const url, NetworkAuthOption con std::string request_url{url}; switch (auth.method) { case AuthMethod::S3PresignedUrlV4: - std::cout << "XWEI before sign: " << request_url << std::endl; if (false == try_sign_url(request_url)) { return nullptr; } - std::cout << "XWEI after sign: " << request_url << std::endl; break; case AuthMethod::None: break; From 2afac89b933ffabf41881676be93545398422476 Mon Sep 17 00:00:00 2001 From: anlowee Date: Tue, 19 Aug 2025 13:33:50 +0000 Subject: [PATCH 08/37] Fix lint --- tools/deployment/presto-clp/scripts/generate-configs.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/generate-configs.py b/tools/deployment/presto-clp/scripts/generate-configs.py index 9fd4317b77..3f64e1bd28 100644 --- a/tools/deployment/presto-clp/scripts/generate-configs.py +++ b/tools/deployment/presto-clp/scripts/generate-configs.py @@ -128,13 +128,17 @@ def _add_clp_env_vars( s3_config_key_prefix = f"archive_output.storage.s3_config" s3_credentials_key_prefix = f"{s3_config_key_prefix}.aws_authentication.credentials" - s3_access_key_id = _get_config_value(clp_config, f"{s3_credentials_key_prefix}.access_key_id") + s3_access_key_id = _get_config_value( + clp_config, f"{s3_credentials_key_prefix}.access_key_id" + ) s3_bucket = _get_config_value(clp_config, f"{s3_config_key_prefix}.bucket") s3_region_code = _get_config_value(clp_config, f"{s3_config_key_prefix}.region_code") s3_end_point = f"https://{s3_bucket}.s3.{s3_region_code}.amazonaws.com/" - s3_secret_access_key = _get_config_value(clp_config, f"{s3_credentials_key_prefix}.secret_access_key") + s3_secret_access_key = _get_config_value( + clp_config, f"{s3_credentials_key_prefix}.secret_access_key" + ) env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"] = "s3" env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER"] = "clp_package" From d7478f807cf6ebe13e391ab6197e438247ea2073 Mon Sep 17 00:00:00 2001 From: anlowee Date: Wed, 20 Aug 2025 17:20:20 +0000 Subject: [PATCH 09/37] Address coderabbitai comments --- ...figs.py => generate-user-env-vars-file.py} | 38 ++++++++----------- .../presto-clp/scripts/set-up-config.sh | 2 +- .../worker/scripts/generate-configs.sh | 23 +++++++++-- 3 files changed, 35 insertions(+), 28 deletions(-) rename tools/deployment/presto-clp/scripts/{generate-configs.py => generate-user-env-vars-file.py} (87%) diff --git a/tools/deployment/presto-clp/scripts/generate-configs.py b/tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py similarity index 87% rename from tools/deployment/presto-clp/scripts/generate-configs.py rename to tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py index 3f64e1bd28..f97f57099a 100644 --- a/tools/deployment/presto-clp/scripts/generate-configs.py +++ b/tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py @@ -50,7 +50,7 @@ def main(argv=None) -> int: clp_config_file_path, clp_package_dir.resolve(), ) - return False + return 1 with open(clp_config_file_path, "r") as clp_config_file: clp_config = yaml.safe_load(clp_config_file) @@ -103,28 +103,15 @@ def _add_clp_env_vars( clp_archive_output_storage_type = _get_config_value( clp_config, "archive_output.storage.type", "fs" ) - if "fs" == clp_archive_output_storage_type: - clp_archives_dir = _get_config_value( - clp_config, - "archive_output.storage.directory", - str(clp_package_dir / "var" / "data" / "archives"), - ) - if Path(clp_archives_dir).is_absolute(): - env_vars["CLP_ARCHIVES_DIR"] = clp_archives_dir - else: - env_vars["CLP_ARCHIVES_DIR"] = str(clp_package_dir / clp_archives_dir) - elif "s3" == clp_archive_output_storage_type: - # This will not be used, just to ensure CLP_ARCHIVES_DIR is not an empty string - clp_archives_dir = _get_config_value( - clp_config, - "archive_output.storage.directory", - str(clp_package_dir / "var" / "data" / "staged-archives"), - ) - if Path(clp_archives_dir).is_absolute(): - env_vars["CLP_ARCHIVES_DIR"] = clp_archives_dir - else: - env_vars["CLP_ARCHIVES_DIR"] = str(clp_package_dir / clp_archives_dir) - + env_vars["CLP_ARCHIVES_DIR"] = _resolve_archives_dir( + clp_package_dir, + _get_config_value(clp_config, "archive_output.storage.directory"), + clp_package_dir + / "var" + / "data" + / ("archives" if clp_archive_output_storage_type == "fs" else "staged-archives"), + ) + if "s3" == clp_archive_output_storage_type: s3_config_key_prefix = f"archive_output.storage.s3_config" s3_credentials_key_prefix = f"{s3_config_key_prefix}.aws_authentication.credentials" @@ -174,6 +161,11 @@ def _add_clp_env_vars( return True +def _resolve_archives_dir(base_dir: Path, configured: Optional[str], default: Path) -> str: + effective = configured or str(default) + return effective if Path(effective).is_absolute() else str(base_dir / effective) + + def _add_worker_env_vars(coordinator_common_env_file_path: Path, env_vars: Dict[str, str]) -> bool: """ Adds environment variables for worker config values to `env_vars`. diff --git a/tools/deployment/presto-clp/scripts/set-up-config.sh b/tools/deployment/presto-clp/scripts/set-up-config.sh index 0b9a8cc546..6c6fb8cced 100755 --- a/tools/deployment/presto-clp/scripts/set-up-config.sh +++ b/tools/deployment/presto-clp/scripts/set-up-config.sh @@ -23,6 +23,6 @@ echo "Installing required Python packages..." pip3 install -r "${script_dir}/requirements.txt" echo "Generating environment variables file for user-configured properties..." -python3 "${script_dir}/generate-configs.py" \ +python3 "${script_dir}/generate-user-env-vars-file.py" \ --clp-package-dir "${clp_package_dir}" \ --output-file "${script_dir}/../.env" diff --git a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh index 8531f55e64..73e4a9fb0c 100755 --- a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh +++ b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh @@ -48,13 +48,19 @@ update_config_file() { local file_path=$1 local key=$2 local value=$3 + local sensitive=${4:-false} if grep --quiet "^${key}=.*$" "$file_path"; then sed --in-place "s|^${key}=.*|${key}=${value}|" "$file_path" else echo "${key}=${value}" >>"$file_path" fi - log "INFO" "Set ${key}=${value} in ${file_path}" + if [[ "$sensitive" == "true" ]]; then + local masked="****${value: -4}" + log "INFO" "Set ${key}=${masked} in ${file_path}" + else + log "INFO" "Set ${key}=${value} in ${file_path}" + fi } apt-get update && apt-get install --assume-yes --no-install-recommends jq wget @@ -76,13 +82,22 @@ mv "${PRESTO_CONFIG_DIR}/clp.properties" "${PRESTO_CONFIG_DIR}/catalog" # Update clp.properties readonly CLP_PROPERTIES_FILE="/opt/presto-server/etc/catalog/clp.properties" -if [ -n "${PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER:-}" ]; then +if [ "${PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE:-}" = "s3" ]; then log "INFO" "Enable S3 support" + for var in PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER \ + PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID \ + PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY \ + PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT; do + if [ -z "${!var:-}" ]; then + log "ERROR" "Missing required env var: $var" + exit 1 + fi + done update_config_file "$CLP_PROPERTIES_FILE" "clp.storage-type" "${PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE}" update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-auth-provider" "${PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER}" - update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-access-key-id" "${PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID}" + update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-access-key-id" "${PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID}" true update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-end-point" "${PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT}" - update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-secret-access-key" "${PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY}" + update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-secret-access-key" "${PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY}" true fi # Update config.properties From 14b3a80f0b00bb1d78861a994cc70dabd3a8b1ab Mon Sep 17 00:00:00 2001 From: anlowee Date: Wed, 20 Aug 2025 17:42:18 +0000 Subject: [PATCH 10/37] Address coderabbitai comments --- .../scripts/generate-user-env-vars-file.py | 22 +++++++++++++++++-- .../worker/scripts/generate-configs.sh | 11 +++++++--- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py b/tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py index f97f57099a..5609a4dc48 100644 --- a/tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py +++ b/tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py @@ -111,7 +111,9 @@ def _add_clp_env_vars( / "data" / ("archives" if clp_archive_output_storage_type == "fs" else "staged-archives"), ) - if "s3" == clp_archive_output_storage_type: + if "fs" == clp_archive_output_storage_type: + pass + elif "s3" == clp_archive_output_storage_type: s3_config_key_prefix = f"archive_output.storage.s3_config" s3_credentials_key_prefix = f"{s3_config_key_prefix}.aws_authentication.credentials" @@ -121,15 +123,31 @@ def _add_clp_env_vars( s3_bucket = _get_config_value(clp_config, f"{s3_config_key_prefix}.bucket") s3_region_code = _get_config_value(clp_config, f"{s3_config_key_prefix}.region_code") - s3_end_point = f"https://{s3_bucket}.s3.{s3_region_code}.amazonaws.com/" s3_secret_access_key = _get_config_value( clp_config, f"{s3_credentials_key_prefix}.secret_access_key" ) + # Validate required S3 fields + missing = [] + + for k, v in { + f"{s3_credentials_key_prefix}.access_key_id": s3_access_key_id, + f"{s3_credentials_key_prefix}.secret_access_key": s3_secret_access_key, + f"{s3_config_key_prefix}.bucket": s3_bucket, + f"{s3_config_key_prefix}.region_code": s3_region_code, + }.items(): + if not v: + missing.append(k) + + if missing: + logger.error("Missing required S3 config key(s): %s", ", ".join(missing)) + return False + env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"] = "s3" env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER"] = "clp_package" env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID"] = s3_access_key_id + s3_end_point = f"https://{s3_bucket}.s3.{s3_region_code}.amazonaws.com/" env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT"] = s3_end_point env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY"] = s3_secret_access_key else: diff --git a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh index 73e4a9fb0c..3db27c2049 100755 --- a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh +++ b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh @@ -56,7 +56,7 @@ update_config_file() { echo "${key}=${value}" >>"$file_path" fi if [[ "$sensitive" == "true" ]]; then - local masked="****${value: -4}" + local masked="****" log "INFO" "Set ${key}=${masked} in ${file_path}" else log "INFO" "Set ${key}=${value} in ${file_path}" @@ -84,15 +84,20 @@ mv "${PRESTO_CONFIG_DIR}/clp.properties" "${PRESTO_CONFIG_DIR}/catalog" readonly CLP_PROPERTIES_FILE="/opt/presto-server/etc/catalog/clp.properties" if [ "${PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE:-}" = "s3" ]; then log "INFO" "Enable S3 support" + missing=() for var in PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER \ PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID \ PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY \ PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT; do if [ -z "${!var:-}" ]; then - log "ERROR" "Missing required env var: $var" - exit 1 + missing+=("$var") fi done + if [ ${#missing[@]} -gt 0 ]; then + log "ERROR" "Missing required env var(s): ${missing[*]}" + exit 1 + fi + update_config_file "$CLP_PROPERTIES_FILE" "clp.storage-type" "${PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE}" update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-auth-provider" "${PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER}" update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-access-key-id" "${PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID}" true From d59af37cc1d5a5c488cc0bba3f241b3554cfe921 Mon Sep 17 00:00:00 2001 From: anlowee Date: Wed, 20 Aug 2025 18:19:52 +0000 Subject: [PATCH 11/37] Update docs to remove the limitation that only local file system files can be queried --- docs/src/user-guide/guides-using-presto.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/src/user-guide/guides-using-presto.md b/docs/src/user-guide/guides-using-presto.md index c68434dcfb..ccb02dd401 100644 --- a/docs/src/user-guide/guides-using-presto.md +++ b/docs/src/user-guide/guides-using-presto.md @@ -164,7 +164,6 @@ The Presto CLP integration has the following limitations at present: * Nested fields containing special characters cannot be queried (see [y-scope/presto#8]). Allowed characters are alphanumeric characters and underscores. To get around this limitation, you'll need to preprocess your logs to remove any special characters. -* Only logs stored on the filesystem, rather than S3, can be queried through Presto. These limitations will be addressed in a future release of the Presto integration. From ada03bbac0769d68fe55687577884d1d5ef01ce9 Mon Sep 17 00:00:00 2001 From: anlowee Date: Mon, 25 Aug 2025 19:13:10 +0000 Subject: [PATCH 12/37] Generate clp.properties by pythong script --- ...generate-user-env-vars-file.py => init.py} | 21 ++++++++++++++++ .../presto-clp/scripts/set-up-config.sh | 2 +- .../worker/scripts/generate-configs.sh | 25 ------------------- 3 files changed, 22 insertions(+), 26 deletions(-) rename tools/deployment/presto-clp/scripts/{generate-user-env-vars-file.py => init.py} (88%) diff --git a/tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py b/tools/deployment/presto-clp/scripts/init.py similarity index 88% rename from tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py rename to tools/deployment/presto-clp/scripts/init.py index 5609a4dc48..77b1fe22d8 100644 --- a/tools/deployment/presto-clp/scripts/generate-user-env-vars-file.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -63,6 +63,9 @@ def main(argv=None) -> int: if not _add_worker_env_vars(script_dir.parent / "coordinator-common.env", env_vars): return 1 + if not _generate_worker_clp_properties(script_dir.parent / "worker" / "config-template", env_vars): + return 1 + with open(output_file, "w") as output_file_handle: for key, value in env_vars.items(): output_file_handle.write(f"{key}={value}\n") @@ -210,6 +213,24 @@ def _add_worker_env_vars(coordinator_common_env_file_path: Path, env_vars: Dict[ return True +def _generate_worker_clp_properties(worker_config_template_path: Path, env_vars: Dict[str, str]) -> bool: + """ + Generates a clp.properties for worker config. + + :param worker_config_template_path: + """ + clp_properties_path = worker_config_template_path / "clp.properties" + config_options = ["connector.name=clp"] + if "s3" == env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"]: + config_options.append("clp.storage-type=s3") + config_options.append(f'clp.s3-auth-provider={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER"]}') + config_options.append(f'clp.s3-access-key-id={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID"]}') + config_options.append(f'clp.s3-end-point={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT"]}') + config_options.append(f'clp.s3-secret-access-key={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY"]}') + with clp_properties_path.open("w", encoding="utf-8") as f: + f.write("\n".join(config_options) + "\n") + + def _get_config_value(config: dict, key: str, default_value: Optional[str] = None) -> str: """ Gets the value corresponding to `key` from `config` if it exists. diff --git a/tools/deployment/presto-clp/scripts/set-up-config.sh b/tools/deployment/presto-clp/scripts/set-up-config.sh index 6c6fb8cced..c92d8c1225 100755 --- a/tools/deployment/presto-clp/scripts/set-up-config.sh +++ b/tools/deployment/presto-clp/scripts/set-up-config.sh @@ -23,6 +23,6 @@ echo "Installing required Python packages..." pip3 install -r "${script_dir}/requirements.txt" echo "Generating environment variables file for user-configured properties..." -python3 "${script_dir}/generate-user-env-vars-file.py" \ +python3 "${script_dir}/init.py" \ --clp-package-dir "${clp_package_dir}" \ --output-file "${script_dir}/../.env" diff --git a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh index 3db27c2049..d3896c19e8 100755 --- a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh +++ b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh @@ -80,31 +80,6 @@ done rm -f "${PRESTO_CONFIG_DIR}/catalog/"* mv "${PRESTO_CONFIG_DIR}/clp.properties" "${PRESTO_CONFIG_DIR}/catalog" -# Update clp.properties -readonly CLP_PROPERTIES_FILE="/opt/presto-server/etc/catalog/clp.properties" -if [ "${PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE:-}" = "s3" ]; then - log "INFO" "Enable S3 support" - missing=() - for var in PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER \ - PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID \ - PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY \ - PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT; do - if [ -z "${!var:-}" ]; then - missing+=("$var") - fi - done - if [ ${#missing[@]} -gt 0 ]; then - log "ERROR" "Missing required env var(s): ${missing[*]}" - exit 1 - fi - - update_config_file "$CLP_PROPERTIES_FILE" "clp.storage-type" "${PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE}" - update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-auth-provider" "${PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER}" - update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-access-key-id" "${PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID}" true - update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-end-point" "${PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT}" - update_config_file "$CLP_PROPERTIES_FILE" "clp.s3-secret-access-key" "${PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY}" true -fi - # Update config.properties readonly CONFIG_PROPERTIES_FILE="/opt/presto-server/etc/config.properties" version=$(get_coordinator_version "$CONFIG_PROPERTIES_FILE") From 968c29c4050da0e1907a6436f2b840bb160a6d09 Mon Sep 17 00:00:00 2001 From: anlowee Date: Mon, 25 Aug 2025 20:01:00 +0000 Subject: [PATCH 13/37] Lint fix --- tools/deployment/presto-clp/scripts/init.py | 38 +++++++++++++++++---- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index 77b1fe22d8..c2111218ce 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -63,7 +63,9 @@ def main(argv=None) -> int: if not _add_worker_env_vars(script_dir.parent / "coordinator-common.env", env_vars): return 1 - if not _generate_worker_clp_properties(script_dir.parent / "worker" / "config-template", env_vars): + if not _generate_worker_clp_properties( + script_dir.parent / "worker" / "config-template", env_vars + ): return 1 with open(output_file, "w") as output_file_handle: @@ -213,22 +215,46 @@ def _add_worker_env_vars(coordinator_common_env_file_path: Path, env_vars: Dict[ return True -def _generate_worker_clp_properties(worker_config_template_path: Path, env_vars: Dict[str, str]) -> bool: +def _generate_worker_clp_properties( + worker_config_template_path: Path, env_vars: Dict[str, str] +) -> bool: """ Generates a clp.properties for worker config. :param worker_config_template_path: + :param env_vars: + :return: Whether the worker clp.properties file was successfully generated. """ clp_properties_path = worker_config_template_path / "clp.properties" config_options = ["connector.name=clp"] if "s3" == env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"]: + required_keys = [ + "PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER", + "PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID", + "PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT", + "PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY", + ] + + for key in required_keys: + if not env_vars.get(key): + return False + config_options.append("clp.storage-type=s3") - config_options.append(f'clp.s3-auth-provider={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER"]}') - config_options.append(f'clp.s3-access-key-id={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID"]}') - config_options.append(f'clp.s3-end-point={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT"]}') - config_options.append(f'clp.s3-secret-access-key={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY"]}') + config_options.append( + f'clp.s3-auth-provider={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER"]}' + ) + config_options.append( + f'clp.s3-access-key-id={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID"]}' + ) + config_options.append( + f'clp.s3-end-point={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT"]}' + ) + config_options.append( + f'clp.s3-secret-access-key={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY"]}' + ) with clp_properties_path.open("w", encoding="utf-8") as f: f.write("\n".join(config_options) + "\n") + return True def _get_config_value(config: dict, key: str, default_value: Optional[str] = None) -> str: From cdaf3f5e1908d02fe9aca08a4caf1f040297889d Mon Sep 17 00:00:00 2001 From: anlowee Date: Mon, 25 Aug 2025 20:16:08 +0000 Subject: [PATCH 14/37] Update the docs and config --- docs/src/user-guide/guides-using-presto.md | 10 ++++++---- tools/deployment/presto-clp/coordinator.env | 1 + .../coordinator/config-template/clp.properties | 3 ++- .../{metadata-filter.json => split-filter.json} | 0 4 files changed, 9 insertions(+), 5 deletions(-) rename tools/deployment/presto-clp/coordinator/config-template/{metadata-filter.json => split-filter.json} (100%) diff --git a/docs/src/user-guide/guides-using-presto.md b/docs/src/user-guide/guides-using-presto.md index ccb02dd401..e162b1dcd3 100644 --- a/docs/src/user-guide/guides-using-presto.md +++ b/docs/src/user-guide/guides-using-presto.md @@ -78,7 +78,7 @@ Using Presto with CLP requires: 4. Configure Presto to use CLP's metadata database as follows: - * Open and edit `coordinator/config-template/metadata-filter.json`. + * Open and edit `coordinator/config-template/split-filter.json`. * For each dataset you want to query, add a filter config of the form: ```json @@ -86,9 +86,11 @@ Using Presto with CLP requires: "clp.default.": [ { "columnName": "", - "rangeMapping": { - "lowerBound": "begin_timestamp", - "upperBound": "end_timestamp" + "customOptions": { + "rangeMapping": { + "lowerBound": "begin_timestamp", + "upperBound": "end_timestamp" + } }, "required": false } diff --git a/tools/deployment/presto-clp/coordinator.env b/tools/deployment/presto-clp/coordinator.env index c246fc7003..21d275fa18 100644 --- a/tools/deployment/presto-clp/coordinator.env +++ b/tools/deployment/presto-clp/coordinator.env @@ -1,5 +1,6 @@ # clp.properties PRESTO_COORDINATOR_CLPPROPERTIES_METADATA_PROVIDER_TYPE=mysql +PRESTO_COORDINATOR_CLPPROPERTIES_SPLIT_FILTER_PROVIDER_TYPE=mysql PRESTO_COORDINATOR_CLPPROPERTIES_SPLIT_PROVIDER=mysql # config.properties diff --git a/tools/deployment/presto-clp/coordinator/config-template/clp.properties b/tools/deployment/presto-clp/coordinator/config-template/clp.properties index cefee52d39..b1e27b026c 100644 --- a/tools/deployment/presto-clp/coordinator/config-template/clp.properties +++ b/tools/deployment/presto-clp/coordinator/config-template/clp.properties @@ -6,4 +6,5 @@ clp.metadata-db-user=${PRESTO_COORDINATOR_CLPPROPERTIES_METADATA_DATABASE_USER} clp.metadata-db-password=${PRESTO_COORDINATOR_CLPPROPERTIES_METADATA_DATABASE_PASSWORD} clp.metadata-table-prefix=${PRESTO_COORDINATOR_CLPPROPERTIES_METADATA_TABLE_PREFIX} clp.split-provider-type=${PRESTO_COORDINATOR_CLPPROPERTIES_SPLIT_PROVIDER} -clp.metadata-filter-config=/opt/presto-server/etc/metadata-filter.json +clp.split-filter-provider-type=${PRESTO_COORDINATOR_CLPPROPERTIES_SPLIT_FILTER_PROVIDER_TYPE} +clp.split-filter-config=/opt/presto-server/etc/split-filter.json diff --git a/tools/deployment/presto-clp/coordinator/config-template/metadata-filter.json b/tools/deployment/presto-clp/coordinator/config-template/split-filter.json similarity index 100% rename from tools/deployment/presto-clp/coordinator/config-template/metadata-filter.json rename to tools/deployment/presto-clp/coordinator/config-template/split-filter.json From e5e1ed63273806fc6b3799e3323509767c2a54fa Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 06:27:05 -0400 Subject: [PATCH 15/37] Refactor _generate_worker_clp_properties. --- tools/deployment/presto-clp/scripts/init.py | 47 +++++++++------------ 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index c2111218ce..ba6013be66 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -219,41 +219,32 @@ def _generate_worker_clp_properties( worker_config_template_path: Path, env_vars: Dict[str, str] ) -> bool: """ - Generates a clp.properties for worker config. + Generates a clp.properties file for the worker. :param worker_config_template_path: :param env_vars: - :return: Whether the worker clp.properties file was successfully generated. + :return: Whether the clp.properties file was successfully generated. """ - clp_properties_path = worker_config_template_path / "clp.properties" - config_options = ["connector.name=clp"] + properties = ["connector.name=clp"] if "s3" == env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"]: - required_keys = [ - "PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER", - "PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID", - "PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT", - "PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY", - ] - - for key in required_keys: - if not env_vars.get(key): + property_name_to_env_var_name = { + "clp.storage-type": "PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE", + "clp.s3-auth-provider": "PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER", + "clp.s3-access-key-id": "PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID", + "clp.s3-end-point": "PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT", + "clp.s3-secret-access-key": "PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY", + } + + for property_name, env_var_name in property_name_to_env_var_name.items(): + env_var_value = env_vars.get(env_var_name) + if not env_var_value: + logger.error("Internal error: Missing required env var '%s'", env_var_name) return False + properties.append(f"{property_name}={env_var_value}") + + with open(worker_config_template_path / "clp.properties", "w", encoding="utf-8") as f: + f.write("\n".join(properties) + "\n") - config_options.append("clp.storage-type=s3") - config_options.append( - f'clp.s3-auth-provider={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER"]}' - ) - config_options.append( - f'clp.s3-access-key-id={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID"]}' - ) - config_options.append( - f'clp.s3-end-point={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT"]}' - ) - config_options.append( - f'clp.s3-secret-access-key={env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY"]}' - ) - with clp_properties_path.open("w", encoding="utf-8") as f: - f.write("\n".join(config_options) + "\n") return True From b9a898fe8b68df1426233e54701ba59b5576626d Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 06:34:46 -0400 Subject: [PATCH 16/37] Remove worker's clp.properties since it'll be generated. --- tools/deployment/presto-clp/.gitignore | 1 + .../deployment/presto-clp/worker/config-template/clp.properties | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 tools/deployment/presto-clp/.gitignore delete mode 100644 tools/deployment/presto-clp/worker/config-template/clp.properties diff --git a/tools/deployment/presto-clp/.gitignore b/tools/deployment/presto-clp/.gitignore new file mode 100644 index 0000000000..50e6da86f2 --- /dev/null +++ b/tools/deployment/presto-clp/.gitignore @@ -0,0 +1 @@ +/worker/config-template/clp.properties diff --git a/tools/deployment/presto-clp/worker/config-template/clp.properties b/tools/deployment/presto-clp/worker/config-template/clp.properties deleted file mode 100644 index 4ec6f9a4c7..0000000000 --- a/tools/deployment/presto-clp/worker/config-template/clp.properties +++ /dev/null @@ -1 +0,0 @@ -connector.name=clp From e012fb005aacd2a920bc67f6cd353aa17f0ce744 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 06:35:53 -0400 Subject: [PATCH 17/37] Undo unnecessary changes in generate-configs.sh. --- .../presto-clp/worker/scripts/generate-configs.sh | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh index d3896c19e8..c04ae86182 100755 --- a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh +++ b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh @@ -48,19 +48,13 @@ update_config_file() { local file_path=$1 local key=$2 local value=$3 - local sensitive=${4:-false} if grep --quiet "^${key}=.*$" "$file_path"; then sed --in-place "s|^${key}=.*|${key}=${value}|" "$file_path" else echo "${key}=${value}" >>"$file_path" fi - if [[ "$sensitive" == "true" ]]; then - local masked="****" - log "INFO" "Set ${key}=${masked} in ${file_path}" - else - log "INFO" "Set ${key}=${value} in ${file_path}" - fi + log "INFO" "Set ${key}=${value} in ${file_path}" } apt-get update && apt-get install --assume-yes --no-install-recommends jq wget From 354a1aa7e8f465a32aa999438f158568ce224fc6 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 07:18:44 -0400 Subject: [PATCH 18/37] Refactor s3 config reading. --- tools/deployment/presto-clp/scripts/init.py | 62 ++++++++++++--------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index ba6013be66..c168b21018 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -119,34 +119,28 @@ def _add_clp_env_vars( if "fs" == clp_archive_output_storage_type: pass elif "s3" == clp_archive_output_storage_type: - s3_config_key_prefix = f"archive_output.storage.s3_config" - s3_credentials_key_prefix = f"{s3_config_key_prefix}.aws_authentication.credentials" - s3_access_key_id = _get_config_value( - clp_config, f"{s3_credentials_key_prefix}.access_key_id" - ) - - s3_bucket = _get_config_value(clp_config, f"{s3_config_key_prefix}.bucket") - s3_region_code = _get_config_value(clp_config, f"{s3_config_key_prefix}.region_code") - - s3_secret_access_key = _get_config_value( - clp_config, f"{s3_credentials_key_prefix}.secret_access_key" - ) - - # Validate required S3 fields - missing = [] - - for k, v in { - f"{s3_credentials_key_prefix}.access_key_id": s3_access_key_id, - f"{s3_credentials_key_prefix}.secret_access_key": s3_secret_access_key, - f"{s3_config_key_prefix}.bucket": s3_bucket, - f"{s3_config_key_prefix}.region_code": s3_region_code, - }.items(): - if not v: - missing.append(k) + try: + s3_config_key = f"archive_output.storage.s3_config" + s3_bucket = _get_required_config_value(clp_config, f"{s3_config_key}.bucket") + s3_region_code = _get_required_config_value(clp_config, f"{s3_config_key}.region_code") + + aws_auth_key = f"{s3_config_key}.aws_authentication" + aws_auth_type_key = f"{aws_auth_key}.type" + aws_auth_type = _get_required_config_value(clp_config, aws_auth_type_key) + credentials_auth_type_str = "credentials" + if credentials_auth_type_str != aws_auth_type: + logger.error("%s for %s is unsupported.", aws_auth_type, aws_auth_type_key) + return False - if missing: - logger.error("Missing required S3 config key(s): %s", ", ".join(missing)) + s3_credentials_key = f"{aws_auth_key}.{credentials_auth_type_str}" + s3_access_key_id = _get_required_config_value( + clp_config, f"{s3_credentials_key}.access_key_id" + ) + s3_secret_access_key = _get_config_value( + clp_config, f"{s3_credentials_key}.secret_access_key" + ) + except KeyError: return False env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"] = "s3" @@ -248,6 +242,22 @@ def _generate_worker_clp_properties( return True +def _get_required_config_value(config: Dict[str, str], key: str) -> str: + """ + Gets the value corresponding to `key` from `config`. Logs an error on failure. + + :param config: The config. + :param key: The key to look for in the config, in dot notation (e.g., "database.host"). + :return: The value corresponding to `key`. + :raises KeyError: If `key` doesn't exist in `config` or its value is `None`. + """ + value = _get_config_value(config, key) + if value is None: + logger.error("Required config '%s' is missing or null.", key) + raise KeyError(key) + return value + + def _get_config_value(config: dict, key: str, default_value: Optional[str] = None) -> str: """ Gets the value corresponding to `key` from `config` if it exists. From d1aa25b36122143d0c6efd0a455003c00e58b9ed Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 07:35:59 -0400 Subject: [PATCH 19/37] Refactor path resolution. Use correct key for staging_directory. --- tools/deployment/presto-clp/scripts/init.py | 50 ++++++++++++++++----- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index c168b21018..7fc184d6b0 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -108,17 +108,25 @@ def _add_clp_env_vars( clp_archive_output_storage_type = _get_config_value( clp_config, "archive_output.storage.type", "fs" ) - env_vars["CLP_ARCHIVES_DIR"] = _resolve_archives_dir( - clp_package_dir, - _get_config_value(clp_config, "archive_output.storage.directory"), - clp_package_dir - / "var" - / "data" - / ("archives" if clp_archive_output_storage_type == "fs" else "staged-archives"), - ) + archive_output_storage_key = "archive_output.storage" if "fs" == clp_archive_output_storage_type: - pass + env_vars["CLP_ARCHIVES_DIR"] = str( + _get_path_clp_config_value( + clp_config, + f"{archive_output_storage_key}.directory", + Path("var") / "data" / "archives", + clp_package_dir + ) + ) elif "s3" == clp_archive_output_storage_type: + env_vars["CLP_ARCHIVES_DIR"] = str( + _get_path_clp_config_value( + clp_config, + f"{archive_output_storage_key}.staging_directory", + Path("var") / "data" / "staged-archives", + clp_package_dir + ) + ) try: s3_config_key = f"archive_output.storage.s3_config" @@ -130,7 +138,7 @@ def _add_clp_env_vars( aws_auth_type = _get_required_config_value(clp_config, aws_auth_type_key) credentials_auth_type_str = "credentials" if credentials_auth_type_str != aws_auth_type: - logger.error("%s for %s is unsupported.", aws_auth_type, aws_auth_type_key) + logger.error("'%s' for %s is unsupported.", aws_auth_type, aws_auth_type_key) return False s3_credentials_key = f"{aws_auth_key}.{credentials_auth_type_str}" @@ -151,9 +159,9 @@ def _add_clp_env_vars( env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY"] = s3_secret_access_key else: logger.error( - "Expected CLP's archive_output.storage.type to be fs or s3 but found '%s'. Presto" - " currently only supports reading archives from the fs or s3 storage type.", + "'%s' for %s is unsupported.", clp_archive_output_storage_type, + archive_output_storage_key ) return False @@ -242,6 +250,24 @@ def _generate_worker_clp_properties( return True +def _get_path_clp_config_value(clp_config: Dict[str, str], key: str, default_value: Path, clp_package_dir: Path) -> Path: + """ + Gets the value corresponding to `key` from `clp_config` as a `Path`. + + :param clp_config: + :param key: The key to look for in the config, in dot notation (e.g., "database.host"). + :param default_value: The value to return if `key` doesn't exist in `config`. + :return: The value corresponding to `key` as a `Path`. Relative paths are returned as + `clp_package_dir` / . + """ + value = _get_config_value(clp_config, key, str(default_value)) + value_as_path = Path(value) + if value_as_path.is_absolute(): + return value_as_path + else: + return clp_package_dir / value_as_path + + def _get_required_config_value(config: Dict[str, str], key: str) -> str: """ Gets the value corresponding to `key` from `config`. Logs an error on failure. From 29c42327788f8b90608e37209f29a7743e7abbe3 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 07:39:30 -0400 Subject: [PATCH 20/37] Extract CLP S3 env var extraction. --- tools/deployment/presto-clp/scripts/init.py | 70 ++++++++++++--------- 1 file changed, 42 insertions(+), 28 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index 7fc184d6b0..ae76fec36a 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -128,35 +128,8 @@ def _add_clp_env_vars( ) ) - try: - s3_config_key = f"archive_output.storage.s3_config" - s3_bucket = _get_required_config_value(clp_config, f"{s3_config_key}.bucket") - s3_region_code = _get_required_config_value(clp_config, f"{s3_config_key}.region_code") - - aws_auth_key = f"{s3_config_key}.aws_authentication" - aws_auth_type_key = f"{aws_auth_key}.type" - aws_auth_type = _get_required_config_value(clp_config, aws_auth_type_key) - credentials_auth_type_str = "credentials" - if credentials_auth_type_str != aws_auth_type: - logger.error("'%s' for %s is unsupported.", aws_auth_type, aws_auth_type_key) - return False - - s3_credentials_key = f"{aws_auth_key}.{credentials_auth_type_str}" - s3_access_key_id = _get_required_config_value( - clp_config, f"{s3_credentials_key}.access_key_id" - ) - s3_secret_access_key = _get_config_value( - clp_config, f"{s3_credentials_key}.secret_access_key" - ) - except KeyError: + if not _add_clp_s3_env_vars(clp_config, env_vars): return False - - env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"] = "s3" - env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER"] = "clp_package" - env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID"] = s3_access_key_id - s3_end_point = f"https://{s3_bucket}.s3.{s3_region_code}.amazonaws.com/" - env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT"] = s3_end_point - env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY"] = s3_secret_access_key else: logger.error( "'%s' for %s is unsupported.", @@ -186,6 +159,47 @@ def _add_clp_env_vars( return True +def _add_clp_s3_env_vars(clp_config: Dict[str, Any], env_vars: Dict[str, str]) -> bool: + """ + Adds environment variables for CLP S3 config values to `env_vars`. + + :param clp_config: + :param env_vars: + :return: Whether the environment variables were successfully added. + """ + try: + s3_config_key = f"archive_output.storage.s3_config" + s3_bucket = _get_required_config_value(clp_config, f"{s3_config_key}.bucket") + s3_region_code = _get_required_config_value(clp_config, f"{s3_config_key}.region_code") + + aws_auth_key = f"{s3_config_key}.aws_authentication" + aws_auth_type_key = f"{aws_auth_key}.type" + aws_auth_type = _get_required_config_value(clp_config, aws_auth_type_key) + credentials_auth_type_str = "credentials" + if credentials_auth_type_str != aws_auth_type: + logger.error("'%s' for %s is unsupported.", aws_auth_type, aws_auth_type_key) + return False + + s3_credentials_key = f"{aws_auth_key}.{credentials_auth_type_str}" + s3_access_key_id = _get_required_config_value( + clp_config, f"{s3_credentials_key}.access_key_id" + ) + s3_secret_access_key = _get_config_value( + clp_config, f"{s3_credentials_key}.secret_access_key" + ) + except KeyError: + return False + + env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"] = "s3" + env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER"] = "clp_package" + env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID"] = s3_access_key_id + s3_end_point = f"https://{s3_bucket}.s3.{s3_region_code}.amazonaws.com/" + env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_END_POINT"] = s3_end_point + env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_SECRET_ACCESS_KEY"] = s3_secret_access_key + + return True + + def _resolve_archives_dir(base_dir: Path, configured: Optional[str], default: Path) -> str: effective = configured or str(default) return effective if Path(effective).is_absolute() else str(base_dir / effective) From c4bcb8b8fd2b2343f1ead7f49bdd24a23d8588fc Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 07:39:59 -0400 Subject: [PATCH 21/37] Remove obsolete method. --- tools/deployment/presto-clp/scripts/init.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index ae76fec36a..53070c48eb 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -200,11 +200,6 @@ def _add_clp_s3_env_vars(clp_config: Dict[str, Any], env_vars: Dict[str, str]) - return True -def _resolve_archives_dir(base_dir: Path, configured: Optional[str], default: Path) -> str: - effective = configured or str(default) - return effective if Path(effective).is_absolute() else str(base_dir / effective) - - def _add_worker_env_vars(coordinator_common_env_file_path: Path, env_vars: Dict[str, str]) -> bool: """ Adds environment variables for worker config values to `env_vars`. From 5cb13b9b6634a14d79a1f43a446f8adf2356db0e Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 07:51:18 -0400 Subject: [PATCH 22/37] Apply linter. --- tools/deployment/presto-clp/scripts/init.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index 53070c48eb..347fc73abe 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -115,7 +115,7 @@ def _add_clp_env_vars( clp_config, f"{archive_output_storage_key}.directory", Path("var") / "data" / "archives", - clp_package_dir + clp_package_dir, ) ) elif "s3" == clp_archive_output_storage_type: @@ -124,7 +124,7 @@ def _add_clp_env_vars( clp_config, f"{archive_output_storage_key}.staging_directory", Path("var") / "data" / "staged-archives", - clp_package_dir + clp_package_dir, ) ) @@ -134,7 +134,7 @@ def _add_clp_env_vars( logger.error( "'%s' for %s is unsupported.", clp_archive_output_storage_type, - archive_output_storage_key + archive_output_storage_key, ) return False @@ -259,7 +259,9 @@ def _generate_worker_clp_properties( return True -def _get_path_clp_config_value(clp_config: Dict[str, str], key: str, default_value: Path, clp_package_dir: Path) -> Path: +def _get_path_clp_config_value( + clp_config: Dict[str, str], key: str, default_value: Path, clp_package_dir: Path +) -> Path: """ Gets the value corresponding to `key` from `clp_config` as a `Path`. From 570c18a076c3bffdd948e008d8b10181663e7eeb Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 07:51:43 -0400 Subject: [PATCH 23/37] Edit set-up-config.sh. --- tools/deployment/presto-clp/scripts/set-up-config.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/deployment/presto-clp/scripts/set-up-config.sh b/tools/deployment/presto-clp/scripts/set-up-config.sh index c92d8c1225..9259dd9f9b 100755 --- a/tools/deployment/presto-clp/scripts/set-up-config.sh +++ b/tools/deployment/presto-clp/scripts/set-up-config.sh @@ -22,7 +22,7 @@ source "${script_dir}/.venv/bin/activate" echo "Installing required Python packages..." pip3 install -r "${script_dir}/requirements.txt" -echo "Generating environment variables file for user-configured properties..." +echo "Generating config files corresponding to user-configured properties..." python3 "${script_dir}/init.py" \ --clp-package-dir "${clp_package_dir}" \ --output-file "${script_dir}/../.env" From 9842ef618f450dab8886d973a6679a6847d90867 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 07:53:20 -0400 Subject: [PATCH 24/37] Note how to configure S3 config and known issue in docs. --- docs/src/user-guide/guides-using-presto.md | 33 ++++++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/docs/src/user-guide/guides-using-presto.md b/docs/src/user-guide/guides-using-presto.md index e162b1dcd3..9ffae6dafb 100644 --- a/docs/src/user-guide/guides-using-presto.md +++ b/docs/src/user-guide/guides-using-presto.md @@ -55,7 +55,14 @@ Using Presto with CLP requires: deployment infrastructure. ::: -3. Continue following the [quick-start](./quick-start/index.md#using-clp) guide to start CLP and +3. If you'd like to store your compressed logs on S3, follow the + [using object storage](guides-using-object-storage/index.md) guide. + + :::{note} + Currently, the Presto integration only supports the + [credentials](guides-using-object-storage/clp-config.md#credentials) authentication type. + +4. Continue following the [quick-start](./quick-start/index.md#using-clp) guide to start CLP and compress your logs. A sample dataset that works well with Presto is [postgresql]. ### Setting up Presto @@ -145,13 +152,31 @@ Each dataset in CLP shows up as a table in Presto. To show all available dataset SHOW TABLES; ``` +:::{note} If you didn't specify a dataset when compressing your logs in CLP, your logs will have been stored -in the `default` dataset. To query the logs in this dataset: +in the `default` dataset. +::: + +To show all available columns in the `default` dataset: + +```sql +DESCRIBE default; +``` + +If you wish to show the columns of a different dataset, replace `default` above. + +To query the logs in this dataset: ```sql -SELECT * FROM default LIMIT 1; +SELECT user FROM default LIMIT 1; ``` +:::{warning} +`SELECT *` currently causes a crash due to a [known issue][y-scope/velox#27]. This will be resolved +soon. See the [limitations](#limitations) section for all current limitations. +::: + + All kv-pairs in each log event can be queried directly using dot-notation. For example, if your logs contain the field `foo.bar`, you can query it using: @@ -163,6 +188,7 @@ SELECT foo.bar FROM default LIMIT 1; The Presto CLP integration has the following limitations at present: +* `SELECT *` currently causes a crash due to a [known issue][y-scope/velox#27]. * Nested fields containing special characters cannot be queried (see [y-scope/presto#8]). Allowed characters are alphanumeric characters and underscores. To get around this limitation, you'll need to preprocess your logs to remove any special characters. @@ -176,4 +202,5 @@ These limitations will be addressed in a future release of the Presto integratio [postgresql]: https://zenodo.org/records/10516401 [Presto]: https://prestodb.io/ [y-scope/presto#8]: https://github.com/y-scope/presto/issues/8 +[y-scope/velox#27]: https://github.com/y-scope/velox/issues/27 [yscope-presto]: https://github.com/y-scope/presto From f7692826f977a596e818ffae76b91dc46541b5e0 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 08:37:29 -0400 Subject: [PATCH 25/37] Remove blank line. --- docs/src/user-guide/guides-using-presto.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/src/user-guide/guides-using-presto.md b/docs/src/user-guide/guides-using-presto.md index 9ffae6dafb..ee38055a37 100644 --- a/docs/src/user-guide/guides-using-presto.md +++ b/docs/src/user-guide/guides-using-presto.md @@ -176,7 +176,6 @@ SELECT user FROM default LIMIT 1; soon. See the [limitations](#limitations) section for all current limitations. ::: - All kv-pairs in each log event can be queried directly using dot-notation. For example, if your logs contain the field `foo.bar`, you can query it using: From a388247436274901c9ae83dce335169bbc3d74f3 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 08:38:39 -0400 Subject: [PATCH 26/37] Fix: Require secret_access_key. --- tools/deployment/presto-clp/scripts/init.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index 347fc73abe..d9598ef568 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -184,7 +184,7 @@ def _add_clp_s3_env_vars(clp_config: Dict[str, Any], env_vars: Dict[str, str]) - s3_access_key_id = _get_required_config_value( clp_config, f"{s3_credentials_key}.access_key_id" ) - s3_secret_access_key = _get_config_value( + s3_secret_access_key = _get_required_config_value( clp_config, f"{s3_credentials_key}.secret_access_key" ) except KeyError: From 72ae05b40b982dba5f19844a51c5c1489bfa5c2c Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 2 Sep 2025 08:40:14 -0400 Subject: [PATCH 27/37] Use correct type annotations. --- tools/deployment/presto-clp/scripts/init.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index d9598ef568..e17d9e763f 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -260,7 +260,7 @@ def _generate_worker_clp_properties( def _get_path_clp_config_value( - clp_config: Dict[str, str], key: str, default_value: Path, clp_package_dir: Path + clp_config: Dict[str, Any], key: str, default_value: Path, clp_package_dir: Path ) -> Path: """ Gets the value corresponding to `key` from `clp_config` as a `Path`. @@ -279,7 +279,7 @@ def _get_path_clp_config_value( return clp_package_dir / value_as_path -def _get_required_config_value(config: Dict[str, str], key: str) -> str: +def _get_required_config_value(config: Dict[str, Any], key: str) -> str: """ Gets the value corresponding to `key` from `config`. Logs an error on failure. From ccaf9a3d912ebc5175f53df146ccd66f84ef7731 Mon Sep 17 00:00:00 2001 From: anlowee Date: Tue, 2 Sep 2025 09:34:22 -0400 Subject: [PATCH 28/37] Address coderabbitai comments --- tools/deployment/presto-clp/scripts/init.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index e17d9e763f..6ae8f53295 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -267,9 +267,9 @@ def _get_path_clp_config_value( :param clp_config: :param key: The key to look for in the config, in dot notation (e.g., "database.host"). - :param default_value: The value to return if `key` doesn't exist in `config`. - :return: The value corresponding to `key` as a `Path`. Relative paths are returned as - `clp_package_dir` / . + :param default_value: Value to use if the key is missing. + :param clp_package_dir: Base for resolving relative paths from the config. + :return: The value as a Path; relative values resolve to clp_package_dir / . """ value = _get_config_value(clp_config, key, str(default_value)) value_as_path = Path(value) @@ -295,7 +295,7 @@ def _get_required_config_value(config: Dict[str, Any], key: str) -> str: return value -def _get_config_value(config: dict, key: str, default_value: Optional[str] = None) -> str: +def _get_config_value(config: dict, key: str, default_value: Optional[str] = None) -> Optional[str]: """ Gets the value corresponding to `key` from `config` if it exists. From b3694b7a84688d34e716a4c9f9b46d0e51b34bdf Mon Sep 17 00:00:00 2001 From: anlowee Date: Tue, 2 Sep 2025 09:41:30 -0400 Subject: [PATCH 29/37] Fix a bug --- tools/deployment/presto-clp/scripts/init.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index 6ae8f53295..010b663a0b 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -237,7 +237,7 @@ def _generate_worker_clp_properties( :return: Whether the clp.properties file was successfully generated. """ properties = ["connector.name=clp"] - if "s3" == env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"]: + if "s3" == env_vars.get("PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"): property_name_to_env_var_name = { "clp.storage-type": "PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE", "clp.s3-auth-provider": "PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER", From 6bcacfbb833853def6cd24a1e7029f1bae596d1f Mon Sep 17 00:00:00 2001 From: anlowee Date: Tue, 2 Sep 2025 14:43:30 -0400 Subject: [PATCH 30/37] Update issue --- docs/src/user-docs/guides-using-presto.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/src/user-docs/guides-using-presto.md b/docs/src/user-docs/guides-using-presto.md index ee38055a37..c9b85b0ca6 100644 --- a/docs/src/user-docs/guides-using-presto.md +++ b/docs/src/user-docs/guides-using-presto.md @@ -172,7 +172,7 @@ SELECT user FROM default LIMIT 1; ``` :::{warning} -`SELECT *` currently causes a crash due to a [known issue][y-scope/velox#27]. This will be resolved +`SELECT *` currently causes a crash due to a [known issue][y-scope/velox#28]. This will be resolved soon. See the [limitations](#limitations) section for all current limitations. ::: @@ -201,5 +201,5 @@ These limitations will be addressed in a future release of the Presto integratio [postgresql]: https://zenodo.org/records/10516401 [Presto]: https://prestodb.io/ [y-scope/presto#8]: https://github.com/y-scope/presto/issues/8 -[y-scope/velox#27]: https://github.com/y-scope/velox/issues/27 +[y-scope/velox#28]: https://github.com/y-scope/velox/issues/28 [yscope-presto]: https://github.com/y-scope/presto From 33592c9ecf76f5fc7971134950a4560ad37c98d5 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Wed, 3 Sep 2025 05:06:57 -0400 Subject: [PATCH 31/37] docs: Change supported release to clp v0.5.0; Remove SELECT * warning; Fix other minor issues. --- docs/src/user-docs/guides-using-presto.md | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/docs/src/user-docs/guides-using-presto.md b/docs/src/user-docs/guides-using-presto.md index c9b85b0ca6..f894d5f0a4 100644 --- a/docs/src/user-docs/guides-using-presto.md +++ b/docs/src/user-docs/guides-using-presto.md @@ -16,7 +16,7 @@ been merged into the main Presto repository so that you can use official Presto ## Requirements -* [CLP][clp-releases] (clp-json) v0.4.0 or higher +* [CLP][clp-releases] (clp-json) v0.5.0 or higher * [Docker] v28 or higher * [Docker Compose][docker-compose] v2.20.2 or higher * Python @@ -61,6 +61,7 @@ Using Presto with CLP requires: :::{note} Currently, the Presto integration only supports the [credentials](guides-using-object-storage/clp-config.md#credentials) authentication type. + ::: 4. Continue following the [quick-start](./quick-start/index.md#using-clp) guide to start CLP and compress your logs. A sample dataset that works well with Presto is [postgresql]. @@ -117,7 +118,7 @@ Using Presto with CLP requires: docker compose up ``` - * To use more than Presto worker, you can use the `--scale` option as follows: + * To use more than one Presto worker, you can use the `--scale` option as follows: ```bash docker compose up --scale presto-worker= @@ -168,14 +169,9 @@ If you wish to show the columns of a different dataset, replace `default` above. To query the logs in this dataset: ```sql -SELECT user FROM default LIMIT 1; +SELECT * FROM default LIMIT 1; ``` -:::{warning} -`SELECT *` currently causes a crash due to a [known issue][y-scope/velox#28]. This will be resolved -soon. See the [limitations](#limitations) section for all current limitations. -::: - All kv-pairs in each log event can be queried directly using dot-notation. For example, if your logs contain the field `foo.bar`, you can query it using: @@ -187,7 +183,6 @@ SELECT foo.bar FROM default LIMIT 1; The Presto CLP integration has the following limitations at present: -* `SELECT *` currently causes a crash due to a [known issue][y-scope/velox#27]. * Nested fields containing special characters cannot be queried (see [y-scope/presto#8]). Allowed characters are alphanumeric characters and underscores. To get around this limitation, you'll need to preprocess your logs to remove any special characters. @@ -201,5 +196,4 @@ These limitations will be addressed in a future release of the Presto integratio [postgresql]: https://zenodo.org/records/10516401 [Presto]: https://prestodb.io/ [y-scope/presto#8]: https://github.com/y-scope/presto/issues/8 -[y-scope/velox#28]: https://github.com/y-scope/velox/issues/28 [yscope-presto]: https://github.com/y-scope/presto From bc86d609364a9275b1f4d89fff9940254ab2de86 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Wed, 3 Sep 2025 05:07:35 -0400 Subject: [PATCH 32/37] Set PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE in both fs and s3 cases for clarity. --- tools/deployment/presto-clp/scripts/init.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index 010b663a0b..5a77bf89be 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -109,6 +109,7 @@ def _add_clp_env_vars( clp_config, "archive_output.storage.type", "fs" ) archive_output_storage_key = "archive_output.storage" + env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"] = clp_archive_output_storage_type if "fs" == clp_archive_output_storage_type: env_vars["CLP_ARCHIVES_DIR"] = str( _get_path_clp_config_value( @@ -190,7 +191,6 @@ def _add_clp_s3_env_vars(clp_config: Dict[str, Any], env_vars: Dict[str, str]) - except KeyError: return False - env_vars["PRESTO_WORKER_CLPPROPERTIES_STORAGE_TYPE"] = "s3" env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_AUTH_PROVIDER"] = "clp_package" env_vars["PRESTO_WORKER_CLPPROPERTIES_S3_ACCESS_KEY_ID"] = s3_access_key_id s3_end_point = f"https://{s3_bucket}.s3.{s3_region_code}.amazonaws.com/" From 3402f9c4f3ab2e1ea2b5482620cff3e794368977 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Wed, 3 Sep 2025 05:43:59 -0400 Subject: [PATCH 33/37] Fix type annotation for _get_config_value. --- tools/deployment/presto-clp/scripts/init.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index 5a77bf89be..88cccca660 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -98,7 +98,7 @@ def _add_clp_env_vars( return False database_host = _get_config_value(clp_config, "database.host", "localhost") - database_port = _get_config_value(clp_config, "database.port", str(3306)) + database_port = _get_config_value(clp_config, "database.port", 3306) database_name = _get_config_value(clp_config, "database.name", "clp-db") env_vars["PRESTO_COORDINATOR_CLPPROPERTIES_METADATA_DATABASE_URL"] = ( f"jdbc:mysql://{database_host}:{database_port}" @@ -295,7 +295,9 @@ def _get_required_config_value(config: Dict[str, Any], key: str) -> str: return value -def _get_config_value(config: dict, key: str, default_value: Optional[str] = None) -> Optional[str]: +def _get_config_value( + config: Dict[str, Any], key: str, default_value: Optional[Any] = None +) -> Optional[Any]: """ Gets the value corresponding to `key` from `config` if it exists. From 2ac911a012346c15fc2fd116947accd63a356bc7 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Wed, 3 Sep 2025 05:45:54 -0400 Subject: [PATCH 34/37] Use _get_required_config_value for database credentials; Add config file path to error message. --- tools/deployment/presto-clp/scripts/init.py | 48 ++++++++++++++------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index 88cccca660..c68ed7bb0f 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -56,7 +56,7 @@ def main(argv=None) -> int: clp_config = yaml.safe_load(clp_config_file) env_vars: Dict[str, str] = {} - if not _add_clp_env_vars(clp_config, clp_package_dir, env_vars): + if not _add_clp_env_vars(clp_config, clp_config_file_path, clp_package_dir, env_vars): return 1 script_dir = Path(__file__).parent.resolve() @@ -76,12 +76,16 @@ def main(argv=None) -> int: def _add_clp_env_vars( - clp_config: Dict[str, Any], clp_package_dir: Path, env_vars: Dict[str, str] + clp_config: Dict[str, Any], + clp_config_file_path: Path, + clp_package_dir: Path, + env_vars: Dict[str, str], ) -> bool: """ Adds environment variables for CLP config values to `env_vars`. :param clp_config: + :param clp_config_file_path: Path to the file containing `clp_config`, for logging. :param clp_package_dir: :param env_vars: :return: Whether the environment variables were successfully added. @@ -129,7 +133,7 @@ def _add_clp_env_vars( ) ) - if not _add_clp_s3_env_vars(clp_config, env_vars): + if not _add_clp_s3_env_vars(clp_config, clp_config_file_path, env_vars): return False else: logger.error( @@ -147,12 +151,14 @@ def _add_clp_env_vars( with open(credentials_file_path, "r") as credentials_file: credentials = yaml.safe_load(credentials_file) - database_user = _get_config_value(credentials, "database.user") - database_password = _get_config_value(credentials, "database.password") - if not database_user or not database_password: - logger.error( - "database.user and database.password must be specified in '%s'.", credentials_file_path + try: + database_user = _get_required_config_value( + credentials, "database.user", credentials_file_path ) + database_password = _get_required_config_value( + credentials, "database.password", credentials_file_path + ) + except KeyError: return False env_vars["PRESTO_COORDINATOR_CLPPROPERTIES_METADATA_DATABASE_USER"] = database_user env_vars["PRESTO_COORDINATOR_CLPPROPERTIES_METADATA_DATABASE_PASSWORD"] = database_password @@ -160,22 +166,31 @@ def _add_clp_env_vars( return True -def _add_clp_s3_env_vars(clp_config: Dict[str, Any], env_vars: Dict[str, str]) -> bool: +def _add_clp_s3_env_vars( + clp_config: Dict[str, Any], clp_config_file_path: Path, env_vars: Dict[str, str] +) -> bool: """ Adds environment variables for CLP S3 config values to `env_vars`. :param clp_config: + :param clp_config_file_path: Path to the file containing `clp_config`, for logging. :param env_vars: :return: Whether the environment variables were successfully added. """ try: s3_config_key = f"archive_output.storage.s3_config" - s3_bucket = _get_required_config_value(clp_config, f"{s3_config_key}.bucket") - s3_region_code = _get_required_config_value(clp_config, f"{s3_config_key}.region_code") + s3_bucket = _get_required_config_value( + clp_config, f"{s3_config_key}.bucket", clp_config_file_path + ) + s3_region_code = _get_required_config_value( + clp_config, f"{s3_config_key}.region_code", clp_config_file_path + ) aws_auth_key = f"{s3_config_key}.aws_authentication" aws_auth_type_key = f"{aws_auth_key}.type" - aws_auth_type = _get_required_config_value(clp_config, aws_auth_type_key) + aws_auth_type = _get_required_config_value( + clp_config, aws_auth_type_key, clp_config_file_path + ) credentials_auth_type_str = "credentials" if credentials_auth_type_str != aws_auth_type: logger.error("'%s' for %s is unsupported.", aws_auth_type, aws_auth_type_key) @@ -183,10 +198,10 @@ def _add_clp_s3_env_vars(clp_config: Dict[str, Any], env_vars: Dict[str, str]) - s3_credentials_key = f"{aws_auth_key}.{credentials_auth_type_str}" s3_access_key_id = _get_required_config_value( - clp_config, f"{s3_credentials_key}.access_key_id" + clp_config, f"{s3_credentials_key}.access_key_id", clp_config_file_path ) s3_secret_access_key = _get_required_config_value( - clp_config, f"{s3_credentials_key}.secret_access_key" + clp_config, f"{s3_credentials_key}.secret_access_key", clp_config_file_path ) except KeyError: return False @@ -279,18 +294,19 @@ def _get_path_clp_config_value( return clp_package_dir / value_as_path -def _get_required_config_value(config: Dict[str, Any], key: str) -> str: +def _get_required_config_value(config: Dict[str, Any], key: str, config_file_path: Path) -> str: """ Gets the value corresponding to `key` from `config`. Logs an error on failure. :param config: The config. :param key: The key to look for in the config, in dot notation (e.g., "database.host"). + :param config_file_path: The path to the config file, for logging. :return: The value corresponding to `key`. :raises KeyError: If `key` doesn't exist in `config` or its value is `None`. """ value = _get_config_value(config, key) if value is None: - logger.error("Required config '%s' is missing or null.", key) + logger.error("Required config '%s' is missing or null in '%s'.", key, config_file_path) raise KeyError(key) return value From 06b5771934abcb3d09d72de341c3b30fe7df519f Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Wed, 3 Sep 2025 05:47:16 -0400 Subject: [PATCH 35/37] Minor touch-up. --- tools/deployment/presto-clp/scripts/init.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index c68ed7bb0f..8cc33abf54 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -283,8 +283,8 @@ def _get_path_clp_config_value( :param clp_config: :param key: The key to look for in the config, in dot notation (e.g., "database.host"). :param default_value: Value to use if the key is missing. - :param clp_package_dir: Base for resolving relative paths from the config. - :return: The value as a Path; relative values resolve to clp_package_dir / . + :param clp_package_dir: Base directory for resolving relative paths in `clp_config`. + :return: The value as a `Path`; relative values resolve to `clp_package_dir` / . """ value = _get_config_value(clp_config, key, str(default_value)) value_as_path = Path(value) From bc90d7f84de0074cfbf02c51f5378e8714e8f455 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Wed, 3 Sep 2025 06:24:38 -0400 Subject: [PATCH 36/37] Update link to split config file syntax. --- docs/src/user-docs/guides-using-presto.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/user-docs/guides-using-presto.md b/docs/src/user-docs/guides-using-presto.md index f894d5f0a4..8929f43574 100644 --- a/docs/src/user-docs/guides-using-presto.md +++ b/docs/src/user-docs/guides-using-presto.md @@ -189,7 +189,7 @@ The Presto CLP integration has the following limitations at present: These limitations will be addressed in a future release of the Presto integration. -[clp-connector-docs]: https://docs.yscope.com/presto/connector/clp.html#metadata-filter-config-file +[clp-connector-docs]: https://docs.yscope.com/presto/connector/clp.html#split-filter-config-file [clp-releases]: https://github.com/y-scope/clp/releases [docker-compose]: https://docs.docker.com/compose/install/ [Docker]: https://docs.docker.com/engine/install/ From 8ac0b0360938851eabfc9665cdae2a25d659dc81 Mon Sep 17 00:00:00 2001 From: anlowee Date: Wed, 3 Sep 2025 10:50:55 -0400 Subject: [PATCH 37/37] Address comments --- docs/src/user-docs/guides-using-presto.md | 2 +- tools/deployment/presto-clp/scripts/init.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/src/user-docs/guides-using-presto.md b/docs/src/user-docs/guides-using-presto.md index 8929f43574..48ea7a00a8 100644 --- a/docs/src/user-docs/guides-using-presto.md +++ b/docs/src/user-docs/guides-using-presto.md @@ -31,7 +31,7 @@ Using Presto with CLP requires: ### Setting up CLP -1. Follow the [quick-start](./quick-start/index.md) guide to download and extract the CLP package, +1. Follow the [quick-start](quick-start/index.md) guide to download and extract the CLP package, but don't start the package just yet. 2. Before starting the package, update the package's config as follows: diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index 8cc33abf54..f2148e6d2f 100644 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -232,7 +232,7 @@ def _add_worker_env_vars(coordinator_common_env_file_path: Path, env_vars: Dict[ ) except KeyError as e: logger.error( - "Missing required key '%s' in '%s'", + "Missing required key %s in '%s'", e, coordinator_common_env_file_path, )