-
Notifications
You must be signed in to change notification settings - Fork 92
feat: Replace existing webui codebase with new-webui. #939
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
8136032
b35a7c2
c87b15c
b169168
ec07bfd
1c89296
a1fb38c
5e473ad
df73718
ad7444e
369a21a
7b2f7a2
f655fad
2a74251
1a7d10c
03435ff
b6f17e1
5a8ddea
78c78d3
bcff918
15a3cdb
c220002
7cae4fa
4d3d114
f269ee6
a34b350
8d4f8ca
34504b2
c1579f4
248347c
ff626c7
efed22a
1697d23
1ec9ff0
e2890f9
a6e46a6
60b4089
b7a6e51
1094e7f
6a499af
b8db62e
4ec47d3
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,7 +24,6 @@ | |||||||||
| CONTROLLER_TARGET_NAME, | ||||||||||
| DB_COMPONENT_NAME, | ||||||||||
| FILES_TABLE_SUFFIX, | ||||||||||
| LOG_VIEWER_WEBUI_COMPONENT_NAME, | ||||||||||
| QUERY_JOBS_TABLE_NAME, | ||||||||||
| QUERY_SCHEDULER_COMPONENT_NAME, | ||||||||||
| QUERY_WORKER_COMPONENT_NAME, | ||||||||||
|
|
@@ -57,7 +56,6 @@ | |||||||||
| validate_and_load_queue_credentials_file, | ||||||||||
| validate_and_load_redis_credentials_file, | ||||||||||
| validate_db_config, | ||||||||||
| validate_log_viewer_webui_config, | ||||||||||
| validate_queue_config, | ||||||||||
| validate_redis_config, | ||||||||||
| validate_reducer_config, | ||||||||||
|
|
@@ -843,132 +841,62 @@ def read_and_update_settings_json(settings_file_path: pathlib.Path, updates: Dic | |||||||||
| return settings_object | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def start_webui(instance_id: str, clp_config: CLPConfig, mounts: CLPDockerMounts): | ||||||||||
| def start_webui( | ||||||||||
| instance_id: str, | ||||||||||
| clp_config: CLPConfig, | ||||||||||
| container_clp_config: CLPConfig, | ||||||||||
| mounts: CLPDockerMounts, | ||||||||||
| ): | ||||||||||
| component_name = WEBUI_COMPONENT_NAME | ||||||||||
| logger.info(f"Starting {component_name}...") | ||||||||||
|
|
||||||||||
| container_name = f"clp-{component_name}-{instance_id}" | ||||||||||
| if container_exists(container_name): | ||||||||||
| return | ||||||||||
|
|
||||||||||
| webui_logs_dir = clp_config.logs_directory / component_name | ||||||||||
| container_webui_dir = CONTAINER_CLP_HOME / "var" / "www" / "webui" | ||||||||||
| node_path = str(container_webui_dir / "programs" / "server" / "npm" / "node_modules") | ||||||||||
| settings_json_path = get_clp_home() / "var" / "www" / "webui" / "settings.json" | ||||||||||
|
|
||||||||||
| validate_webui_config(clp_config, webui_logs_dir, settings_json_path) | ||||||||||
|
|
||||||||||
| # Create directories | ||||||||||
| webui_logs_dir.mkdir(exist_ok=True, parents=True) | ||||||||||
| node_path = str(container_webui_dir / "server" / "node_modules") | ||||||||||
| client_settings_json_path = ( | ||||||||||
| get_clp_home() / "var" / "www" / "webui" / "client" / "settings.json" | ||||||||||
| ) | ||||||||||
| server_settings_json_path = ( | ||||||||||
| get_clp_home() / "var" / "www" / "webui" / "server" / "dist" / "server" / "settings.json" | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| container_webui_logs_dir = pathlib.Path("/") / "var" / "log" / component_name | ||||||||||
| validate_webui_config(clp_config, client_settings_json_path, server_settings_json_path) | ||||||||||
|
|
||||||||||
| # Read and update settings.json | ||||||||||
| # Read, update, and write back client's and server's settings.json | ||||||||||
| clp_db_connection_params = clp_config.database.get_clp_connection_params_and_type(True) | ||||||||||
| table_prefix = clp_db_connection_params["table_prefix"] | ||||||||||
| if StorageEngine.CLP_S == clp_config.package.storage_engine: | ||||||||||
| table_prefix = f"{table_prefix}{CLP_DEFAULT_DATASET_NAME}_" | ||||||||||
| meteor_settings_updates = { | ||||||||||
| "private": { | ||||||||||
| "SqlDbHost": clp_config.database.host, | ||||||||||
| "SqlDbPort": clp_config.database.port, | ||||||||||
| "SqlDbName": clp_config.database.name, | ||||||||||
| "SqlDbClpArchivesTableName": f"{table_prefix}{ARCHIVES_TABLE_SUFFIX}", | ||||||||||
| "SqlDbClpFilesTableName": f"{table_prefix}{FILES_TABLE_SUFFIX}", | ||||||||||
|
Comment on lines
-876
to
-877
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. Since we're using dataset-specific tables for clp-json, we'll need to pass these into the webui from here, right?
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. gg.. sorry for missing this |
||||||||||
| "SqlDbCompressionJobsTableName": COMPRESSION_JOBS_TABLE_NAME, | ||||||||||
| "SqlDbQueryJobsTableName": QUERY_JOBS_TABLE_NAME, | ||||||||||
| }, | ||||||||||
| "public": { | ||||||||||
| "ClpStorageEngine": clp_config.package.storage_engine, | ||||||||||
| "LogViewerWebuiUrl": ( | ||||||||||
| f"http://{clp_config.log_viewer_webui.host}:{clp_config.log_viewer_webui.port}", | ||||||||||
| ), | ||||||||||
| }, | ||||||||||
| client_settings_json_updates = { | ||||||||||
| "ClpStorageEngine": clp_config.package.storage_engine, | ||||||||||
| "MongoDbSearchResultsMetadataCollectionName": clp_config.webui.results_metadata_collection_name, | ||||||||||
|
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 (assertive) Fix line length violations. Lines 869 and 885 exceed the 100-character limit. Consider breaking them into multiple lines for better readability. - "MongoDbSearchResultsMetadataCollectionName": clp_config.webui.results_metadata_collection_name,
+ "MongoDbSearchResultsMetadataCollectionName": (
+ clp_config.webui.results_metadata_collection_name
+ ),- "MongoDbSearchResultsMetadataCollectionName": clp_config.webui.results_metadata_collection_name,
+ "MongoDbSearchResultsMetadataCollectionName": (
+ clp_config.webui.results_metadata_collection_name
+ ),Also applies to: 885-885 🧰 Tools🪛 Pylint (3.3.7)[convention] 869-869: Line too long (104/100) (C0301) 🤖 Prompt for AI Agents |
||||||||||
| "SqlDbClpArchivesTableName": f"{table_prefix}{ARCHIVES_TABLE_SUFFIX}", | ||||||||||
| "SqlDbClpFilesTableName": f"{table_prefix}{FILES_TABLE_SUFFIX}", | ||||||||||
| "SqlDbCompressionJobsTableName": COMPRESSION_JOBS_TABLE_NAME, | ||||||||||
| } | ||||||||||
| meteor_settings = read_and_update_settings_json(settings_json_path, meteor_settings_updates) | ||||||||||
|
|
||||||||||
| # Start container | ||||||||||
| # fmt: off | ||||||||||
| container_cmd = [ | ||||||||||
| "docker", "run", | ||||||||||
| "-d", | ||||||||||
| "--network", "host", | ||||||||||
| "--name", container_name, | ||||||||||
| "--log-driver", "local", | ||||||||||
| "-u", f"{os.getuid()}:{os.getgid()}", | ||||||||||
| ] | ||||||||||
| # fmt: on | ||||||||||
| env_vars = [ | ||||||||||
| f"NODE_PATH={node_path}", | ||||||||||
| f"MONGO_URL={clp_config.results_cache.get_uri()}", | ||||||||||
| f"PORT={clp_config.webui.port}", | ||||||||||
| f"ROOT_URL=http://{clp_config.webui.host}", | ||||||||||
| f"METEOR_SETTINGS={json.dumps(meteor_settings)}", | ||||||||||
| f"CLP_DB_USER={clp_config.database.username}", | ||||||||||
| f"CLP_DB_PASS={clp_config.database.password}", | ||||||||||
| f"WEBUI_LOGS_DIR={container_webui_logs_dir}", | ||||||||||
| f"WEBUI_LOGGING_LEVEL={clp_config.webui.logging_level}", | ||||||||||
| ] | ||||||||||
| necessary_mounts = [ | ||||||||||
| mounts.clp_home, | ||||||||||
| DockerMount(DockerMountType.BIND, webui_logs_dir, container_webui_logs_dir), | ||||||||||
| ] | ||||||||||
| append_docker_options(container_cmd, necessary_mounts, env_vars) | ||||||||||
| container_cmd.append(clp_config.execution_container) | ||||||||||
|
|
||||||||||
| node_cmd = [ | ||||||||||
| str(CONTAINER_CLP_HOME / "bin" / "node-14"), | ||||||||||
| str(container_webui_dir / "launcher.js"), | ||||||||||
| str(container_webui_dir / "main.js"), | ||||||||||
| ] | ||||||||||
| cmd = container_cmd + node_cmd | ||||||||||
| subprocess.run(cmd, stdout=subprocess.DEVNULL, check=True) | ||||||||||
|
|
||||||||||
| logger.info(f"Started {component_name}.") | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def start_log_viewer_webui( | ||||||||||
| instance_id: str, | ||||||||||
| clp_config: CLPConfig, | ||||||||||
| container_clp_config: CLPConfig, | ||||||||||
| mounts: CLPDockerMounts, | ||||||||||
| ): | ||||||||||
| component_name = LOG_VIEWER_WEBUI_COMPONENT_NAME | ||||||||||
| logger.info(f"Starting {component_name}...") | ||||||||||
|
|
||||||||||
| container_name = f"clp-{component_name}-{instance_id}" | ||||||||||
| if container_exists(container_name): | ||||||||||
| return | ||||||||||
|
|
||||||||||
| container_log_viewer_webui_dir = CONTAINER_CLP_HOME / "var" / "www" / "log-viewer-webui" | ||||||||||
| node_path = str(container_log_viewer_webui_dir / "server" / "node_modules") | ||||||||||
| settings_json_path = ( | ||||||||||
| get_clp_home() | ||||||||||
| / "var" | ||||||||||
| / "www" | ||||||||||
| / "log-viewer-webui" | ||||||||||
| / "server" | ||||||||||
| / "dist" | ||||||||||
| / "server" | ||||||||||
| / "settings.json" | ||||||||||
| client_settings_json = read_and_update_settings_json( | ||||||||||
| client_settings_json_path, client_settings_json_updates | ||||||||||
| ) | ||||||||||
| with open(client_settings_json_path, "w") as client_settings_json_file: | ||||||||||
| client_settings_json_file.write(json.dumps(client_settings_json)) | ||||||||||
|
Comment on lines
+883
to
+884
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. 🛠️ Refactor suggestion Specify encoding when opening files. When opening files for reading or writing, it's best practice to explicitly specify the encoding to avoid potential issues with non-ASCII characters. - with open(client_settings_json_path, "w") as client_settings_json_file:
- client_settings_json_file.write(json.dumps(client_settings_json))
+ with open(client_settings_json_path, "w", encoding="utf-8") as client_settings_json_file:
+ client_settings_json_file.write(json.dumps(client_settings_json))- with open(server_settings_json_path, "w") as settings_json_file:
- settings_json_file.write(json.dumps(server_settings_json))
+ with open(server_settings_json_path, "w", encoding="utf-8") as settings_json_file:
+ settings_json_file.write(json.dumps(server_settings_json))Also applies to: 916-917 🧰 Tools🪛 Pylint (3.3.7)[warning] 874-874: Using open without explicitly specifying an encoding (W1514) 🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
| validate_log_viewer_webui_config(clp_config, settings_json_path) | ||||||||||
|
|
||||||||||
| # Read, update, and write back settings.json | ||||||||||
| settings_json_updates = { | ||||||||||
| server_settings_json_updates = { | ||||||||||
| "SqlDbHost": clp_config.database.host, | ||||||||||
| "SqlDbPort": clp_config.database.port, | ||||||||||
| "SqlDbName": clp_config.database.name, | ||||||||||
| "SqlDbQueryJobsTableName": QUERY_JOBS_TABLE_NAME, | ||||||||||
| "MongoDbHost": clp_config.results_cache.host, | ||||||||||
| "MongoDbPort": clp_config.results_cache.port, | ||||||||||
| "MongoDbName": clp_config.results_cache.db_name, | ||||||||||
| "MongoDbSearchResultsMetadataCollectionName": clp_config.webui.results_metadata_collection_name, | ||||||||||
|
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 (assertive) Fix line length violation. This line exceeds the 100-character limit. Consider breaking it into multiple lines: - "MongoDbSearchResultsMetadataCollectionName": clp_config.webui.results_metadata_collection_name,
+ "MongoDbSearchResultsMetadataCollectionName": (
+ clp_config.webui.results_metadata_collection_name
+ ),📝 Committable suggestion
Suggested change
🧰 Tools🪛 Pylint (3.3.7)[convention] 894-894: Line too long (104/100) (C0301) 🤖 Prompt for AI Agents |
||||||||||
| "MongoDbStreamFilesCollectionName": clp_config.results_cache.stream_collection_name, | ||||||||||
| "ClientDir": str(container_log_viewer_webui_dir / "client"), | ||||||||||
| "ClientDir": str(container_webui_dir / "client"), | ||||||||||
| "LogViewerDir": str(container_webui_dir / "yscope-log-viewer"), | ||||||||||
| "StreamFilesDir": str(container_clp_config.stream_output.get_directory()), | ||||||||||
| "StreamTargetUncompressedSize": container_clp_config.stream_output.target_uncompressed_size, | ||||||||||
| "LogViewerDir": str(container_log_viewer_webui_dir / "yscope-log-viewer"), | ||||||||||
| } | ||||||||||
|
|
||||||||||
| container_cmd_extra_opts = [] | ||||||||||
|
|
@@ -977,23 +905,25 @@ def start_log_viewer_webui( | |||||||||
| if StorageType.S3 == stream_storage.type: | ||||||||||
| s3_config = stream_storage.s3_config | ||||||||||
|
|
||||||||||
| settings_json_updates["StreamFilesS3Region"] = s3_config.region_code | ||||||||||
| settings_json_updates["StreamFilesS3PathPrefix"] = ( | ||||||||||
| server_settings_json_updates["StreamFilesS3Region"] = s3_config.region_code | ||||||||||
| server_settings_json_updates["StreamFilesS3PathPrefix"] = ( | ||||||||||
| f"{s3_config.bucket}/{s3_config.key_prefix}" | ||||||||||
| ) | ||||||||||
| auth = s3_config.aws_authentication | ||||||||||
| if AwsAuthType.profile == auth.type: | ||||||||||
| settings_json_updates["StreamFilesS3Profile"] = auth.profile | ||||||||||
| server_settings_json_updates["StreamFilesS3Profile"] = auth.profile | ||||||||||
| else: | ||||||||||
| settings_json_updates["StreamFilesS3Profile"] = None | ||||||||||
| server_settings_json_updates["StreamFilesS3Profile"] = None | ||||||||||
| elif StorageType.FS == stream_storage.type: | ||||||||||
| settings_json_updates["StreamFilesS3Region"] = None | ||||||||||
| settings_json_updates["StreamFilesS3PathPrefix"] = None | ||||||||||
| settings_json_updates["StreamFilesS3Profile"] = None | ||||||||||
| server_settings_json_updates["StreamFilesS3Region"] = None | ||||||||||
| server_settings_json_updates["StreamFilesS3PathPrefix"] = None | ||||||||||
| server_settings_json_updates["StreamFilesS3Profile"] = None | ||||||||||
|
|
||||||||||
| settings_json = read_and_update_settings_json(settings_json_path, settings_json_updates) | ||||||||||
| with open(settings_json_path, "w") as settings_json_file: | ||||||||||
| settings_json_file.write(json.dumps(settings_json)) | ||||||||||
| server_settings_json = read_and_update_settings_json( | ||||||||||
| server_settings_json_path, server_settings_json_updates | ||||||||||
| ) | ||||||||||
| with open(server_settings_json_path, "w") as settings_json_file: | ||||||||||
| settings_json_file.write(json.dumps(server_settings_json)) | ||||||||||
|
|
||||||||||
| # fmt: off | ||||||||||
| container_cmd = [ | ||||||||||
|
|
@@ -1009,8 +939,8 @@ def start_log_viewer_webui( | |||||||||
|
|
||||||||||
| necessary_env_vars = [ | ||||||||||
| f"NODE_PATH={node_path}", | ||||||||||
| f"HOST={clp_config.log_viewer_webui.host}", | ||||||||||
| f"PORT={clp_config.log_viewer_webui.port}", | ||||||||||
| f"HOST={clp_config.webui.host}", | ||||||||||
| f"PORT={clp_config.webui.port}", | ||||||||||
| f"CLP_DB_USER={clp_config.database.username}", | ||||||||||
| f"CLP_DB_PASS={clp_config.database.password}", | ||||||||||
| f"NODE_ENV=production", | ||||||||||
|
|
@@ -1027,7 +957,7 @@ def start_log_viewer_webui( | |||||||||
| necessary_env_vars.append(f"AWS_SECRET_ACCESS_KEY={credentials.secret_access_key}") | ||||||||||
| else: | ||||||||||
| aws_mount, aws_env_vars = generate_container_auth_options( | ||||||||||
| clp_config, LOG_VIEWER_WEBUI_COMPONENT_NAME | ||||||||||
| clp_config, WEBUI_COMPONENT_NAME | ||||||||||
| ) | ||||||||||
| if aws_mount: | ||||||||||
| necessary_mounts.append(mounts.aws_config_dir) | ||||||||||
|
|
@@ -1038,7 +968,7 @@ def start_log_viewer_webui( | |||||||||
|
|
||||||||||
| node_cmd = [ | ||||||||||
| str(CONTAINER_CLP_HOME / "bin" / "node-22"), | ||||||||||
| str(container_log_viewer_webui_dir / "server" / "dist" / "server" / "src" / "main.js"), | ||||||||||
| str(container_webui_dir / "server" / "dist" / "server" / "src" / "main.js"), | ||||||||||
| ] | ||||||||||
| cmd = container_cmd + node_cmd | ||||||||||
| subprocess.run(cmd, stdout=subprocess.DEVNULL, check=True) | ||||||||||
|
|
@@ -1147,7 +1077,6 @@ def main(argv): | |||||||||
| reducer_server_parser = component_args_parser.add_parser(REDUCER_COMPONENT_NAME) | ||||||||||
| add_num_workers_argument(reducer_server_parser) | ||||||||||
| component_args_parser.add_parser(WEBUI_COMPONENT_NAME) | ||||||||||
| component_args_parser.add_parser(LOG_VIEWER_WEBUI_COMPONENT_NAME) | ||||||||||
|
|
||||||||||
| parsed_args = args_parser.parse_args(argv[1:]) | ||||||||||
|
|
||||||||||
|
|
@@ -1175,7 +1104,6 @@ def main(argv): | |||||||||
| COMPRESSION_SCHEDULER_COMPONENT_NAME, | ||||||||||
| QUERY_SCHEDULER_COMPONENT_NAME, | ||||||||||
| WEBUI_COMPONENT_NAME, | ||||||||||
| LOG_VIEWER_WEBUI_COMPONENT_NAME, | ||||||||||
| ): | ||||||||||
| validate_and_load_db_credentials_file(clp_config, clp_home, True) | ||||||||||
| if target in ( | ||||||||||
|
|
@@ -1271,9 +1199,7 @@ def main(argv): | |||||||||
| if target in (ALL_TARGET_NAME, REDUCER_COMPONENT_NAME): | ||||||||||
| start_reducer(instance_id, clp_config, container_clp_config, num_workers, mounts) | ||||||||||
| if target in (ALL_TARGET_NAME, WEBUI_COMPONENT_NAME): | ||||||||||
| start_webui(instance_id, clp_config, mounts) | ||||||||||
| if target in (ALL_TARGET_NAME, LOG_VIEWER_WEBUI_COMPONENT_NAME): | ||||||||||
| start_log_viewer_webui(instance_id, clp_config, container_clp_config, mounts) | ||||||||||
| start_webui(instance_id, clp_config, container_clp_config, mounts) | ||||||||||
|
|
||||||||||
| except Exception as ex: | ||||||||||
| if type(ex) == ValueError: | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Add a docstring to document the function.
The function is missing documentation. Consider adding a docstring that describes the purpose, parameters, and any exceptions that might be raised.
def start_webui( instance_id: str, clp_config: CLPConfig, container_clp_config: CLPConfig, mounts: CLPDockerMounts, ): + """ + Starts the web UI component with both client and server configurations. + + :param instance_id: Unique identifier for this CLP instance. + :param clp_config: Host CLP configuration. + :param container_clp_config: Container-specific CLP configuration. + :param mounts: Docker mount configurations. + :raises ValueError: If settings JSON files are invalid. + """ component_name = WEBUI_COMPONENT_NAME📝 Committable suggestion
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 842-842: Missing function or method docstring
(C0116)
[refactor] 842-842: Too many local variables (28/15)
(R0914)
[refactor] 842-842: Too many statements (55/50)
(R0915)
🤖 Prompt for AI Agents