Skip to content
Merged
17 changes: 14 additions & 3 deletions components/clp-package-utils/clp_package_utils/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,16 @@
API_SERVER_COMPONENT_NAME,
AwsAuthType,
BundledService,
CLP_DB_PASS_ENV_VAR_NAME,
CLP_DB_ROOT_PASS_ENV_VAR_NAME,
CLP_DB_ROOT_USER_ENV_VAR_NAME,
CLP_DB_USER_ENV_VAR_NAME,
ClpConfig,
ClpDbUserType,
COMPRESSION_JOBS_TABLE_NAME,
COMPRESSION_SCHEDULER_COMPONENT_NAME,
COMPRESSION_WORKER_COMPONENT_NAME,
DatabaseEngine,
DB_COMPONENT_NAME,
DeploymentType,
GARBAGE_COLLECTOR_COMPONENT_NAME,
Expand Down Expand Up @@ -150,7 +156,9 @@ def _set_up_env_for_database_bundling(self) -> EnvVarsDict:
# Runtime config
env_vars |= {
"CLP_DB_CONTAINER_IMAGE_REF": (
"mysql:8.0.23" if self._clp_config.database.type == "mysql" else "mariadb:10-jammy"
"mysql:8.0.23"
if self._clp_config.database.type == DatabaseEngine.MYSQL
else "mariadb:10-jammy"
),
}

Expand All @@ -175,9 +183,12 @@ def _set_up_env_for_database(self) -> EnvVarsDict:
}

# Credentials
credentials = self._clp_config.database.credentials
env_vars |= {
"CLP_DB_PASS": self._clp_config.database.password,
"CLP_DB_USER": self._clp_config.database.username,
CLP_DB_PASS_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].password,
CLP_DB_ROOT_PASS_ENV_VAR_NAME: credentials[ClpDbUserType.ROOT].password,
CLP_DB_ROOT_USER_ENV_VAR_NAME: credentials[ClpDbUserType.ROOT].username,
CLP_DB_USER_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].username,
}

return env_vars
Expand Down
7 changes: 6 additions & 1 deletion components/clp-package-utils/clp_package_utils/general.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,12 @@ def load_config_file(

def generate_credentials_file(credentials_file_path: pathlib.Path):
credentials = {
DB_COMPONENT_NAME: {"username": "clp-user", "password": secrets.token_urlsafe(8)},
DB_COMPONENT_NAME: {
"username": "clp-user",
"password": secrets.token_urlsafe(8),
"root_username": "root",
"root_password": secrets.token_urlsafe(8),
},
QUEUE_COMPONENT_NAME: {"username": "clp-user", "password": secrets.token_urlsafe(8)},
REDIS_COMPONENT_NAME: {"password": secrets.token_urlsafe(16)},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
CLP_DB_USER_ENV_VAR_NAME,
CLP_DEFAULT_CONFIG_FILE_RELATIVE_PATH,
CLP_DEFAULT_DATASET_NAME,
ClpDbUserType,
StorageEngine,
StorageType,
)
Expand Down Expand Up @@ -226,9 +227,10 @@ def main(argv: list[str]) -> int:
mounts.logs_dir,
mounts.archives_output_dir,
]
credentials = clp_config.database.credentials
extra_env_vars = {
CLP_DB_USER_ENV_VAR_NAME: clp_config.database.username,
CLP_DB_PASS_ENV_VAR_NAME: clp_config.database.password,
CLP_DB_PASS_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].password,
CLP_DB_USER_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].username,
}
container_start_cmd: list[str] = generate_container_start_cmd(
container_name, necessary_mounts, clp_config.container_image_ref, extra_env_vars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
CLP_DB_USER_ENV_VAR_NAME,
CLP_DEFAULT_CONFIG_FILE_RELATIVE_PATH,
CLP_DEFAULT_DATASET_NAME,
ClpDbUserType,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

CLP credential usage is correct; consider a shared helper for DB env construction

Using credentials[ClpDbUserType.CLP].username/password for CLP_DB_USER_ENV_VAR_NAME / CLP_DB_PASS_ENV_VAR_NAME is consistent with the new per‑user credential model and with how you validate/load credentials earlier in this script, so behaviour here looks correct.

Given the identical pattern now exists in multiple wrappers (compress.py, compress_from_s3.py, search.py, dataset_manager.py, decompress.py, and the native scripts), you may want to factor this into a small helper (e.g., in clp_package_utils.general or a tiny DB‑env utility) that takes a Database or ClpConfig and returns the CLP DB env dict. That would reduce duplication and keep future changes to credential handling in one place.

Also applies to: 256-260

🤖 Prompt for AI Agents
components/clp-package-utils/clp_package_utils/scripts/compress.py lines ~14 and
~256-260: the code repeats constructing CLP DB env vars using
credentials[ClpDbUserType.CLP].username/password across multiple scripts;
extract this duplicated logic into a small helper (e.g.,
clp_package_utils.general or clp_package_utils.db_utils) that accepts the
Database/ClpConfig (or credentials mapping) and returns the CLP DB env dict with
keys CLP_DB_USER_ENV_VAR_NAME and CLP_DB_PASS_ENV_VAR_NAME (and any other shared
DB env entries), then replace the inline construction in compress.py (and the
other listed scripts: compress_from_s3.py, search.py, dataset_manager.py,
decompress.py and native scripts) with calls to that helper to centralize
credential handling and reduce duplication.

StorageEngine,
StorageType,
)
Expand Down Expand Up @@ -252,9 +253,10 @@ def main(argv):
logger.error("No filesystem paths given for compression.")
return -1

credentials = clp_config.database.credentials
extra_env_vars = {
CLP_DB_USER_ENV_VAR_NAME: clp_config.database.username,
CLP_DB_PASS_ENV_VAR_NAME: clp_config.database.password,
CLP_DB_PASS_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].password,
CLP_DB_USER_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].username,
}
container_start_cmd = generate_container_start_cmd(
container_name, necessary_mounts, clp_config.container_image_ref, extra_env_vars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
CLP_DB_USER_ENV_VAR_NAME,
CLP_DEFAULT_CONFIG_FILE_RELATIVE_PATH,
CLP_DEFAULT_DATASET_NAME,
ClpDbUserType,
StorageEngine,
StorageType,
)
Expand Down Expand Up @@ -306,9 +307,10 @@ def main(argv):
logger.error("No S3 URLs given for compression.")
return -1

credentials = clp_config.database.credentials
extra_env_vars = {
CLP_DB_USER_ENV_VAR_NAME: clp_config.database.username,
CLP_DB_PASS_ENV_VAR_NAME: clp_config.database.password,
CLP_DB_PASS_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].password,
CLP_DB_USER_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].username,
}
container_start_cmd = generate_container_start_cmd(
container_name, necessary_mounts, clp_config.container_image_ref, extra_env_vars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
CLP_DB_PASS_ENV_VAR_NAME,
CLP_DB_USER_ENV_VAR_NAME,
CLP_DEFAULT_CONFIG_FILE_RELATIVE_PATH,
ClpDbUserType,
StorageEngine,
StorageType,
)
Expand Down Expand Up @@ -155,9 +156,10 @@ def main(argv: list[str]) -> int:
if aws_mount:
necessary_mounts.append(mounts.aws_config_dir)

credentials = clp_config.database.credentials
extra_env_vars = {
CLP_DB_USER_ENV_VAR_NAME: clp_config.database.username,
CLP_DB_PASS_ENV_VAR_NAME: clp_config.database.password,
CLP_DB_PASS_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].password,
CLP_DB_USER_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].username,
}
container_start_cmd = generate_container_start_cmd(
container_name, necessary_mounts, clp_config.container_image_ref, extra_env_vars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
CLP_DEFAULT_CONFIG_FILE_RELATIVE_PATH,
CLP_DEFAULT_DATASET_NAME,
ClpConfig,
ClpDbUserType,
StorageEngine,
StorageType,
)
Expand Down Expand Up @@ -132,9 +133,10 @@ def handle_extract_file_cmd(
)
)

credentials = clp_config.database.credentials
extra_env_vars = {
CLP_DB_USER_ENV_VAR_NAME: clp_config.database.username,
CLP_DB_PASS_ENV_VAR_NAME: clp_config.database.password,
CLP_DB_PASS_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].password,
CLP_DB_USER_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].username,
}
container_start_cmd = generate_container_start_cmd(
container_name, necessary_mounts, clp_config.container_image_ref, extra_env_vars
Expand Down Expand Up @@ -214,9 +216,10 @@ def handle_extract_stream_cmd(
container_clp_config, clp_config, get_container_config_filename(container_name)
)
necessary_mounts = [mounts.logs_dir]
credentials = clp_config.database.credentials
extra_env_vars = {
CLP_DB_USER_ENV_VAR_NAME: clp_config.database.username,
CLP_DB_PASS_ENV_VAR_NAME: clp_config.database.password,
CLP_DB_PASS_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].password,
CLP_DB_USER_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].username,
}
container_start_cmd = generate_container_start_cmd(
container_name, necessary_mounts, clp_config.container_image_ref, extra_env_vars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
CLP_DB_USER_ENV_VAR_NAME,
CLP_DEFAULT_CONFIG_FILE_RELATIVE_PATH,
ClpConfig,
ClpDbUserType,
Database,
)
from clp_py_utils.clp_metadata_db_utils import get_files_table_name
Expand Down Expand Up @@ -245,10 +246,11 @@ def handle_extract_file_cmd(
"--db-table-prefix", clp_db_connection_params["table_prefix"],
]
# fmt: on
credentials = clp_db_connection_params["credentials"]
extract_env = {
**os.environ,
CLP_DB_USER_ENV_VAR_NAME: clp_db_connection_params["username"],
CLP_DB_PASS_ENV_VAR_NAME: clp_db_connection_params["password"],
CLP_DB_USER_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].username,
CLP_DB_PASS_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].password,
}

files_to_extract_list_path = None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
CLP_DB_USER_ENV_VAR_NAME,
CLP_DEFAULT_CONFIG_FILE_RELATIVE_PATH,
CLP_DEFAULT_DATASET_NAME,
ClpDbUserType,
StorageEngine,
StorageType,
)
Expand Down Expand Up @@ -134,9 +135,10 @@ def main(argv):
container_clp_config, clp_config, get_container_config_filename(container_name)
)
necessary_mounts = [mounts.logs_dir]
credentials = clp_config.database.credentials
extra_env_vars = {
CLP_DB_USER_ENV_VAR_NAME: clp_config.database.username,
CLP_DB_PASS_ENV_VAR_NAME: clp_config.database.password,
CLP_DB_PASS_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].password,
CLP_DB_USER_ENV_VAR_NAME: credentials[ClpDbUserType.CLP].username,
}
container_start_cmd = generate_container_start_cmd(
container_name, necessary_mounts, clp_config.container_image_ref, extra_env_vars
Expand Down
Loading
Loading