-
Notifications
You must be signed in to change notification settings - Fork 83
feat(clp-package)!: Containerize the scripts in sbin via Docker Compose; Remove redundant assets from the package (resolves #1358).
#1512
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
00efbc1
ed3dde2
d307457
f871424
b03539a
1b65afe
3b4d44c
347b181
74a3921
885e9b5
0db2f30
695ed77
4f21e0f
a7c81a3
e6e8217
be2d200
d910944
163921b
3f5c302
65d421f
83c581c
e03a361
299795f
0ccd155
beacdcf
c7141e7
25c2450
0ac4501
479fc81
9921d48
a2dd5cf
8d6deb6
c6c5164
3b44945
4b6f337
bb90b69
ae825c0
9a66669
6983943
e372661
bee1b79
b0e8abf
5172963
bb5d8d2
95a8e1f
395e42f
3af2e4d
198f398
1d02051
3063e85
306997a
4f6c5b4
ef627c6
75ada35
969fde0
732dc92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,7 @@ | |
| get_datasets_table_name, | ||
| get_files_table_name, | ||
| ) | ||
| from clp_py_utils.core import resolve_host_path_in_container | ||
|
|
||
| from clp_package_utils.general import ( | ||
| check_docker_dependencies, | ||
|
|
@@ -119,9 +120,12 @@ def _set_up_env_for_database(self) -> EnvVarsDict: | |
| logs_dir = self._clp_config.logs_directory / component_name | ||
| validate_db_config(self._clp_config, conf_logging_file, data_dir, logs_dir) | ||
|
|
||
| data_dir.mkdir(exist_ok=True, parents=True) | ||
| logs_dir.mkdir(exist_ok=True, parents=True) | ||
| _chown_paths_if_root(data_dir, logs_dir) | ||
| resolved_data_dir = resolve_host_path_in_container(data_dir) | ||
| resolved_logs_dir = resolve_host_path_in_container(logs_dir) | ||
|
|
||
| resolved_data_dir.mkdir(exist_ok=True, parents=True) | ||
| resolved_logs_dir.mkdir(exist_ok=True, parents=True) | ||
| _chown_paths_if_root(resolved_data_dir, resolved_logs_dir) | ||
|
|
||
| env_vars = EnvVarsDict() | ||
|
|
||
|
|
@@ -166,8 +170,9 @@ def _set_up_env_for_queue(self) -> EnvVarsDict: | |
| logs_dir = self._clp_config.logs_directory / component_name | ||
| validate_queue_config(self._clp_config, logs_dir) | ||
|
|
||
| logs_dir.mkdir(exist_ok=True, parents=True) | ||
| _chown_paths_if_root(logs_dir) | ||
| resolved_logs_dir = resolve_host_path_in_container(logs_dir) | ||
| resolved_logs_dir.mkdir(exist_ok=True, parents=True) | ||
| _chown_paths_if_root(resolved_logs_dir) | ||
|
|
||
| env_vars = EnvVarsDict() | ||
|
|
||
|
|
@@ -204,9 +209,12 @@ def _set_up_env_for_redis(self) -> EnvVarsDict: | |
| logs_dir = self._clp_config.logs_directory / component_name | ||
| validate_redis_config(self._clp_config, conf_file, data_dir, logs_dir) | ||
|
|
||
| data_dir.mkdir(exist_ok=True, parents=True) | ||
| logs_dir.mkdir(exist_ok=True, parents=True) | ||
| _chown_paths_if_root(data_dir, logs_dir) | ||
| resolved_data_dir = resolve_host_path_in_container(data_dir) | ||
| resolved_logs_dir = resolve_host_path_in_container(logs_dir) | ||
|
|
||
| resolved_data_dir.mkdir(exist_ok=True, parents=True) | ||
| resolved_logs_dir.mkdir(exist_ok=True, parents=True) | ||
| _chown_paths_if_root(resolved_data_dir, resolved_logs_dir) | ||
|
|
||
| env_vars = EnvVarsDict() | ||
|
|
||
|
|
@@ -252,9 +260,12 @@ def _set_up_env_for_results_cache(self) -> EnvVarsDict: | |
| logs_dir = self._clp_config.logs_directory / component_name | ||
| validate_results_cache_config(self._clp_config, conf_file, data_dir, logs_dir) | ||
|
|
||
| data_dir.mkdir(exist_ok=True, parents=True) | ||
| logs_dir.mkdir(exist_ok=True, parents=True) | ||
| _chown_paths_if_root(data_dir, logs_dir) | ||
| resolved_data_dir = resolve_host_path_in_container(data_dir) | ||
| resolved_logs_dir = resolve_host_path_in_container(logs_dir) | ||
|
|
||
| resolved_data_dir.mkdir(exist_ok=True, parents=True) | ||
| resolved_logs_dir.mkdir(exist_ok=True, parents=True) | ||
| _chown_paths_if_root(resolved_data_dir, resolved_logs_dir) | ||
|
|
||
| env_vars = EnvVarsDict() | ||
|
|
||
|
|
@@ -291,7 +302,8 @@ def _set_up_env_for_compression_scheduler(self) -> EnvVarsDict: | |
| logger.info(f"Setting up environment for {component_name}...") | ||
|
|
||
| logs_dir = self._clp_config.logs_directory / component_name | ||
| logs_dir.mkdir(parents=True, exist_ok=True) | ||
| resolved_logs_dir = resolve_host_path_in_container(logs_dir) | ||
| resolved_logs_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| env_vars = EnvVarsDict() | ||
|
|
||
|
|
@@ -314,7 +326,8 @@ def _set_up_env_for_query_scheduler(self) -> EnvVarsDict: | |
| logger.info(f"Setting up environment for {component_name}...") | ||
|
|
||
| logs_dir = self._clp_config.logs_directory / component_name | ||
| logs_dir.mkdir(parents=True, exist_ok=True) | ||
| resolved_logs_dir = resolve_host_path_in_container(logs_dir) | ||
| resolved_logs_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| env_vars = EnvVarsDict() | ||
|
|
||
|
|
@@ -336,7 +349,8 @@ def _set_up_env_for_compression_worker(self, num_workers: int) -> EnvVarsDict: | |
| logger.info(f"Setting up environment for {component_name}...") | ||
|
|
||
| logs_dir = self._clp_config.logs_directory / component_name | ||
| logs_dir.mkdir(parents=True, exist_ok=True) | ||
| resolved_logs_dir = resolve_host_path_in_container(logs_dir) | ||
| resolved_logs_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| env_vars = EnvVarsDict() | ||
|
|
||
|
|
@@ -365,7 +379,8 @@ def _set_up_env_for_query_worker(self, num_workers: int) -> EnvVarsDict: | |
| logger.info(f"Setting up environment for {component_name}...") | ||
|
|
||
| logs_dir = self._clp_config.logs_directory / component_name | ||
| logs_dir.mkdir(parents=True, exist_ok=True) | ||
| resolved_logs_dir = resolve_host_path_in_container(logs_dir) | ||
| resolved_logs_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| env_vars = EnvVarsDict() | ||
|
|
||
|
|
@@ -392,7 +407,8 @@ def _set_up_env_for_reducer(self, num_workers: int) -> EnvVarsDict: | |
| logger.info(f"Setting up environment for {component_name}...") | ||
|
|
||
| logs_dir = self._clp_config.logs_directory / component_name | ||
| logs_dir.mkdir(parents=True, exist_ok=True) | ||
| resolved_logs_dir = resolve_host_path_in_container(logs_dir) | ||
| resolved_logs_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| env_vars = EnvVarsDict() | ||
|
|
||
|
|
@@ -427,7 +443,9 @@ def _set_up_env_for_webui(self, container_clp_config: CLPConfig) -> EnvVarsDict: | |
| self._clp_home / "var" / "www" / "webui" / "server" / "dist" / "settings.json" | ||
| ) | ||
| validate_webui_config( | ||
| self._clp_config, client_settings_json_path, server_settings_json_path | ||
| self._clp_config, | ||
| client_settings_json_path, | ||
| server_settings_json_path, | ||
| ) | ||
|
|
||
| # Read, update, and write back client's and server's settings.json | ||
|
|
@@ -454,10 +472,13 @@ def _set_up_env_for_webui(self, container_clp_config: CLPConfig) -> EnvVarsDict: | |
| "SqlDbClpTablePrefix": table_prefix, | ||
| "SqlDbCompressionJobsTableName": COMPRESSION_JOBS_TABLE_NAME, | ||
| } | ||
| resolved_client_settings_json_path = resolve_host_path_in_container( | ||
| client_settings_json_path | ||
| ) | ||
| client_settings_json = self._read_and_update_settings_json( | ||
| client_settings_json_path, client_settings_json_updates | ||
| resolved_client_settings_json_path, client_settings_json_updates | ||
| ) | ||
| with open(client_settings_json_path, "w") as client_settings_json_file: | ||
| with open(resolved_client_settings_json_path, "w") as client_settings_json_file: | ||
| client_settings_json_file.write(json.dumps(client_settings_json)) | ||
|
Comment on lines
+479
to
482
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Prefer json.dump with indentation for readability. Small DX improvement for troubleshooting settings. - with open(resolved_client_settings_json_path, "w") as client_settings_json_file:
- client_settings_json_file.write(json.dumps(client_settings_json))
+ with open(resolved_client_settings_json_path, "w") as client_settings_json_file:
+ json.dump(client_settings_json, client_settings_json_file, indent=2, sort_keys=True)
@@
- with open(resolved_server_settings_json_path, "w") as settings_json_file:
- settings_json_file.write(json.dumps(server_settings_json))
+ with open(resolved_server_settings_json_path, "w") as settings_json_file:
+ json.dump(server_settings_json, settings_json_file, indent=2, sort_keys=True)Also applies to: 536-537 🤖 Prompt for AI Agents |
||
|
|
||
| server_settings_json_updates = { | ||
|
|
@@ -509,10 +530,13 @@ def _set_up_env_for_webui(self, container_clp_config: CLPConfig) -> EnvVarsDict: | |
| server_settings_json_updates["PrestoHost"] = None | ||
| server_settings_json_updates["PrestoPort"] = None | ||
|
|
||
| resolved_server_settings_json_path = resolve_host_path_in_container( | ||
| server_settings_json_path | ||
| ) | ||
| server_settings_json = self._read_and_update_settings_json( | ||
| server_settings_json_path, server_settings_json_updates | ||
| resolved_server_settings_json_path, server_settings_json_updates | ||
| ) | ||
| with open(server_settings_json_path, "w") as settings_json_file: | ||
| with open(resolved_server_settings_json_path, "w") as settings_json_file: | ||
| settings_json_file.write(json.dumps(server_settings_json)) | ||
|
|
||
| env_vars = EnvVarsDict() | ||
|
|
@@ -544,7 +568,9 @@ def _set_up_env_for_mcp_server(self) -> EnvVarsDict: | |
|
|
||
| logs_dir = self._clp_config.logs_directory / component_name | ||
| validate_mcp_server_config(self._clp_config, logs_dir) | ||
| logs_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| resolved_logs_dir = resolve_host_path_in_container(logs_dir) | ||
junhaoliao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| resolved_logs_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| env_vars = EnvVarsDict() | ||
|
|
||
|
|
@@ -581,7 +607,8 @@ def _set_up_env_for_garbage_collector(self) -> EnvVarsDict: | |
| logger.info(f"Setting up environment for {component_name}...") | ||
|
|
||
| logs_dir = self._clp_config.logs_directory / component_name | ||
| logs_dir.mkdir(parents=True, exist_ok=True) | ||
| resolved_logs_dir = resolve_host_path_in_container(logs_dir) | ||
| resolved_logs_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| env_vars = EnvVarsDict() | ||
|
|
||
|
|
@@ -812,13 +839,14 @@ def get_or_create_instance_id(clp_config: CLPConfig) -> str: | |
| :return: The instance ID. | ||
| """ | ||
| instance_id_file_path = clp_config.logs_directory / "instance-id" | ||
| resolved_instance_id_file_path = resolve_host_path_in_container(instance_id_file_path) | ||
|
|
||
| if instance_id_file_path.exists(): | ||
| with open(instance_id_file_path, "r") as f: | ||
| if resolved_instance_id_file_path.exists(): | ||
| with open(resolved_instance_id_file_path, "r") as f: | ||
| instance_id = f.readline() | ||
| else: | ||
| instance_id = str(uuid.uuid4())[-4:] | ||
| with open(instance_id_file_path, "w") as f: | ||
| with open(resolved_instance_id_file_path, "w") as f: | ||
| f.write(instance_id) | ||
|
|
||
| return instance_id | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -38,6 +38,7 @@ | |||||||||||||||||
| get_config_value, | ||||||||||||||||||
| make_config_path_absolute, | ||||||||||||||||||
| read_yaml_config_file, | ||||||||||||||||||
| resolve_host_path_in_container, | ||||||||||||||||||
| validate_path_could_be_dir, | ||||||||||||||||||
| ) | ||||||||||||||||||
| from strenum import KebabCaseStrEnum | ||||||||||||||||||
|
|
@@ -355,7 +356,8 @@ def dump_container_config( | |||||||||||||||||
| """ | ||||||||||||||||||
| config_file_path_on_host = clp_config.logs_directory / config_filename | ||||||||||||||||||
| config_file_path_on_container = container_clp_config.logs_directory / config_filename | ||||||||||||||||||
| with open(config_file_path_on_host, "w") as f: | ||||||||||||||||||
| resolved_config_file_path_on_host = resolve_host_path_in_container(config_file_path_on_host) | ||||||||||||||||||
| with open(resolved_config_file_path_on_host, "w") as f: | ||||||||||||||||||
| yaml.safe_dump(container_clp_config.dump_to_primitive_dict(), f) | ||||||||||||||||||
|
|
||||||||||||||||||
| return config_file_path_on_container, config_file_path_on_host | ||||||||||||||||||
|
|
@@ -463,16 +465,17 @@ def validate_credentials_file_path( | |||||||||||||||||
| clp_config: CLPConfig, clp_home: pathlib.Path, generate_default_file: bool | ||||||||||||||||||
| ): | ||||||||||||||||||
| credentials_file_path = clp_config.credentials_file_path | ||||||||||||||||||
| if not credentials_file_path.exists(): | ||||||||||||||||||
| resolved_credentials_file_path = resolve_host_path_in_container(credentials_file_path) | ||||||||||||||||||
| if not resolved_credentials_file_path.exists(): | ||||||||||||||||||
| if ( | ||||||||||||||||||
| make_config_path_absolute(clp_home, CLP_DEFAULT_CREDENTIALS_FILE_PATH) | ||||||||||||||||||
| == credentials_file_path | ||||||||||||||||||
| and generate_default_file | ||||||||||||||||||
| ): | ||||||||||||||||||
| generate_credentials_file(credentials_file_path) | ||||||||||||||||||
| generate_credentials_file(resolved_credentials_file_path) | ||||||||||||||||||
| else: | ||||||||||||||||||
| raise ValueError(f"Credentials file path '{credentials_file_path}' does not exist.") | ||||||||||||||||||
| elif not credentials_file_path.is_file(): | ||||||||||||||||||
| elif not resolved_credentials_file_path.is_file(): | ||||||||||||||||||
| raise ValueError(f"Credentials file path '{credentials_file_path}' is not a file.") | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -503,7 +506,8 @@ def validate_db_config( | |||||||||||||||||
| data_dir: pathlib.Path, | ||||||||||||||||||
| logs_dir: pathlib.Path, | ||||||||||||||||||
| ): | ||||||||||||||||||
| if not component_config.exists(): | ||||||||||||||||||
| resolved_component_config = resolve_host_path_in_container(component_config) | ||||||||||||||||||
| if not resolved_component_config.exists(): | ||||||||||||||||||
| raise ValueError(f"{DB_COMPONENT_NAME} configuration file missing: '{component_config}'.") | ||||||||||||||||||
| _validate_data_directory(data_dir, DB_COMPONENT_NAME) | ||||||||||||||||||
| _validate_log_directory(logs_dir, DB_COMPONENT_NAME) | ||||||||||||||||||
|
|
@@ -523,7 +527,8 @@ def validate_redis_config( | |||||||||||||||||
| data_dir: pathlib.Path, | ||||||||||||||||||
| logs_dir: pathlib.Path, | ||||||||||||||||||
| ): | ||||||||||||||||||
| if not component_config.exists(): | ||||||||||||||||||
| resolved_component_config = resolve_host_path_in_container(component_config) | ||||||||||||||||||
| if not resolved_component_config.exists(): | ||||||||||||||||||
| raise ValueError( | ||||||||||||||||||
| f"{REDIS_COMPONENT_NAME} configuration file missing: '{component_config}'." | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
@@ -550,7 +555,8 @@ def validate_results_cache_config( | |||||||||||||||||
| data_dir: pathlib.Path, | ||||||||||||||||||
| logs_dir: pathlib.Path, | ||||||||||||||||||
| ): | ||||||||||||||||||
| if not component_config.exists(): | ||||||||||||||||||
| resolved_component_config = resolve_host_path_in_container(component_config) | ||||||||||||||||||
| if not resolved_component_config.exists(): | ||||||||||||||||||
| raise ValueError( | ||||||||||||||||||
| f"{RESULTS_CACHE_COMPONENT_NAME} configuration file missing: '{component_config}'." | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
@@ -564,13 +570,9 @@ def validate_results_cache_config( | |||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| def validate_logs_input_config(clp_config: CLPConfig) -> None: | ||||||||||||||||||
| clp_config.validate_logs_input_config() | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| def validate_output_storage_config(clp_config: CLPConfig) -> None: | ||||||||||||||||||
| clp_config.validate_archive_output_config() | ||||||||||||||||||
| clp_config.validate_stream_output_config() | ||||||||||||||||||
| clp_config.validate_archive_output_config(True) | ||||||||||||||||||
| clp_config.validate_stream_output_config(True) | ||||||||||||||||||
|
Comment on lines
571
to
+575
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Pass boolean parameters by keyword for clarity. The positional Apply this diff: - clp_config.validate_archive_output_config(True)
- clp_config.validate_stream_output_config(True)
+ clp_config.validate_archive_output_config(use_host_mount=True)
+ clp_config.validate_stream_output_config(use_host_mount=True)📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.14.2)575-575: Boolean positional value in function call (FBT003) 576-576: Boolean positional value in function call (FBT003) 🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| validate_path_for_container_mount(clp_config.archive_output.get_directory()) | ||||||||||||||||||
| validate_path_for_container_mount(clp_config.stream_output.get_directory()) | ||||||||||||||||||
|
|
@@ -582,7 +584,8 @@ def validate_webui_config( | |||||||||||||||||
| server_settings_json_path: pathlib.Path, | ||||||||||||||||||
| ): | ||||||||||||||||||
| for path in [client_settings_json_path, server_settings_json_path]: | ||||||||||||||||||
| if not path.exists(): | ||||||||||||||||||
| resolved_path = resolve_host_path_in_container(path) | ||||||||||||||||||
| if not resolved_path.exists(): | ||||||||||||||||||
| raise ValueError(f"{WEBUI_COMPONENT_NAME} {path} is not a valid path to settings.json") | ||||||||||||||||||
|
|
||||||||||||||||||
| validate_port(f"{WEBUI_COMPONENT_NAME}.port", clp_config.webui.host, clp_config.webui.port) | ||||||||||||||||||
|
|
@@ -764,7 +767,7 @@ def _is_docker_compose_project_running(project_name: str) -> bool: | |||||||||||||||||
|
|
||||||||||||||||||
| def _validate_data_directory(data_dir: pathlib.Path, component_name: str) -> None: | ||||||||||||||||||
| try: | ||||||||||||||||||
| validate_path_could_be_dir(data_dir) | ||||||||||||||||||
| validate_path_could_be_dir(resolve_host_path_in_container(data_dir)) | ||||||||||||||||||
| except ValueError as ex: | ||||||||||||||||||
| raise ValueError(f"{component_name} data directory is invalid: {ex}") | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -778,6 +781,6 @@ def _validate_log_directory(logs_dir: pathlib.Path, component_name: str): | |||||||||||||||||
| :raise ValueError: If the path is invalid or can't be a directory. | ||||||||||||||||||
| """ | ||||||||||||||||||
| try: | ||||||||||||||||||
| validate_path_could_be_dir(logs_dir) | ||||||||||||||||||
| validate_path_could_be_dir(resolve_host_path_in_container(logs_dir)) | ||||||||||||||||||
| except ValueError as ex: | ||||||||||||||||||
| raise ValueError(f"{component_name} logs directory is invalid: {ex}") | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |||||||
| StorageEngine, | ||||||||
| StorageType, | ||||||||
| ) | ||||||||
| from clp_py_utils.core import resolve_host_path_in_container | ||||||||
|
|
||||||||
| from clp_package_utils.general import ( | ||||||||
| CLPConfig, | ||||||||
|
|
@@ -171,9 +172,11 @@ def main(argv: List[str]) -> int: | |||||||
| try: | ||||||||
| config_file_path: Path = Path(parsed_args.config) | ||||||||
| clp_config: CLPConfig = load_config_file( | ||||||||
| config_file_path, default_config_file_path, clp_home | ||||||||
| resolve_host_path_in_container(config_file_path), | ||||||||
| resolve_host_path_in_container(default_config_file_path), | ||||||||
| clp_home, | ||||||||
| ) | ||||||||
|
Comment on lines
+175
to
178
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard config path resolution to preserve container-local defaults. The current implementation unconditionally translates both For Apply this diff: + import os
+
# Validate and load config file
try:
config_file_path: Path = Path(parsed_args.config)
+ # Check if config_file_path exists in container; if not, resolve it as a host path
+ if config_file_path.exists():
+ resolved_config_file_path = config_file_path
+ else:
+ resolved_config_file_path = resolve_host_path_in_container(config_file_path)
+
+ # For default config, use container path if it exists; otherwise resolve as host path
+ if default_config_file_path.exists():
+ resolved_default_config_file_path = default_config_file_path
+ else:
+ resolved_default_config_file_path = resolve_host_path_in_container(
+ default_config_file_path
+ )
+
clp_config: CLPConfig = load_config_file(
- resolve_host_path_in_container(config_file_path),
- resolve_host_path_in_container(default_config_file_path),
+ resolved_config_file_path,
+ resolved_default_config_file_path,
clp_home,
)Based on learnings. 🤖 Prompt for AI Agents |
||||||||
| clp_config.validate_logs_dir() | ||||||||
| clp_config.validate_logs_dir(True) | ||||||||
|
|
||||||||
|
Comment on lines
+179
to
180
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Pass Keeps intent clear and avoids FBT003. - clp_config.validate_logs_dir(True)
+ clp_config.validate_logs_dir(use_host_mount=True)📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.14.2)179-179: Boolean positional value in function call (FBT003) 🤖 Prompt for AI Agents |
||||||||
| # Validate and load necessary credentials | ||||||||
| validate_and_load_db_credentials_file(clp_config, clp_home, False) | ||||||||
|
|
@@ -220,7 +223,6 @@ def main(argv: List[str]) -> int: | |||||||
| ) | ||||||||
|
|
||||||||
| necessary_mounts: List[Optional[DockerMount]] = [ | ||||||||
| mounts.clp_home, | ||||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mounts like this have to be removed, or they conflict with the contents inside the docker image.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, can you explain more?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry, that comment was so unclear that it took me time to recall why it was done - on the host, clp_home is essentially the clp-package directory root. previously we were mounting the package root into the container so the python modules in |
||||||||
| mounts.logs_dir, | ||||||||
| mounts.archives_output_dir, | ||||||||
| ] | ||||||||
|
|
@@ -281,7 +283,10 @@ def main(argv: List[str]) -> int: | |||||||
| logger.debug(f"Docker command failed: {shlex.join(cmd)}") | ||||||||
|
|
||||||||
| # Remove generated files | ||||||||
| generated_config_path_on_host.unlink() | ||||||||
| resolved_generated_config_path_on_host = resolve_host_path_in_container( | ||||||||
| generated_config_path_on_host | ||||||||
| ) | ||||||||
| resolved_generated_config_path_on_host.unlink() | ||||||||
|
|
||||||||
| return ret_code | ||||||||
|
|
||||||||
|
|
||||||||
Uh oh!
There was an error while loading. Please reload this page.