feat(clp-package): Expose dataset selection in package CLI scripts and web UI. (Deprecated) #1004
Conversation
…eduler can create them on demand
|
@junhaoliao - Adding you to review web code. I guess @kirkrodrigues will review package |
Bill-hbrhbr
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (1)
9-9: Duplicate deep import suggestion
Same as above: consider replacing../../../api/sqlwith a path alias or absolute import for clarity and maintainability.components/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsx (1)
10-10: Duplicate deep import suggestion
Same deep-import pattern here; switching to a configured alias (e.g.@api/sql) would reduce churn when directory structures change.components/clp-package-utils/clp_package_utils/scripts/native/search.py (1)
220-234: Consistent function call parameter passing.The dataset parameter is correctly passed in the
run_function_in_processcall, maintaining consistency with the previous implementation pattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
components/log-viewer-webui/client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (34)
components/clp-package-utils/clp_package_utils/scripts/archive_manager.py(3 hunks)components/clp-package-utils/clp_package_utils/scripts/compress.py(3 hunks)components/clp-package-utils/clp_package_utils/scripts/decompress.py(3 hunks)components/clp-package-utils/clp_package_utils/scripts/native/archive_manager.py(4 hunks)components/clp-package-utils/clp_package_utils/scripts/native/compress.py(4 hunks)components/clp-package-utils/clp_package_utils/scripts/native/decompress.py(6 hunks)components/clp-package-utils/clp_package_utils/scripts/native/search.py(10 hunks)components/clp-package-utils/clp_package_utils/scripts/search.py(3 hunks)components/log-viewer-webui/client/package.json(1 hunks)components/log-viewer-webui/client/src/AntdApp.tsx(2 hunks)components/log-viewer-webui/client/src/api/query.ts(1 hunks)components/log-viewer-webui/client/src/api/search.ts(1 hunks)components/log-viewer-webui/client/src/api/sql.ts(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/sqlConfig.ts(0 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.module.css(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/index.module.css(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton.tsx(0 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/index.tsx(2 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx(3 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsx(3 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsx(6 hunks)components/log-viewer-webui/client/src/typings/query.ts(1 hunks)components/log-viewer-webui/client/src/ui/QueryStatus.tsx(2 hunks)components/log-viewer-webui/server/src/fastify-v2/routes/api/search/index.ts(2 hunks)components/log-viewer-webui/server/src/fastify-v2/schemas/search.ts(1 hunks)components/log-viewer-webui/server/src/plugins/DbManager.ts(2 hunks)components/log-viewer-webui/server/src/routes/query.ts(2 hunks)
💤 Files with no reviewable changes (2)
- components/log-viewer-webui/client/src/pages/IngestPage/sqlConfig.ts
- components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton.tsx
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/api/sql.tscomponents/log-viewer-webui/client/src/AntdApp.tsxcomponents/log-viewer-webui/client/src/ui/QueryStatus.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsxcomponents/log-viewer-webui/client/src/api/search.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/index.tsxcomponents/log-viewer-webui/server/src/routes/query.tscomponents/log-viewer-webui/server/src/fastify-v2/schemas/search.tscomponents/log-viewer-webui/client/src/typings/query.tscomponents/log-viewer-webui/server/src/fastify-v2/routes/api/search/index.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsxcomponents/log-viewer-webui/server/src/plugins/DbManager.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsxcomponents/log-viewer-webui/client/src/api/query.ts
🪛 Pylint (3.3.7)
components/clp-package-utils/clp_package_utils/scripts/compress.py
[error] 9-12: Unable to import 'clp_py_utils.clp_config'
(E0401)
components/clp-package-utils/clp_package_utils/scripts/native/compress.py
[error] 13-17: Unable to import 'clp_py_utils.clp_config'
(E0401)
components/clp-package-utils/clp_package_utils/scripts/archive_manager.py
[error] 8-11: Unable to import 'clp_py_utils.clp_config'
(E0401)
components/clp-package-utils/clp_package_utils/scripts/search.py
[error] 10-14: Unable to import 'clp_py_utils.clp_config'
(E0401)
components/clp-package-utils/clp_package_utils/scripts/decompress.py
[error] 8-13: Unable to import 'clp_py_utils.clp_config'
(E0401)
components/clp-package-utils/clp_package_utils/scripts/native/search.py
[error] 13-17: Unable to import 'clp_py_utils.clp_config'
(E0401)
🔇 Additional comments (56)
components/log-viewer-webui/client/package.json (1)
22-23: Ensure lockfile update
The newly added React Query packages require updating your lockfile (e.g.yarn.lockorpackage-lock.json) to lock their versions.components/log-viewer-webui/server/src/fastify-v2/schemas/search.ts (1)
13-13: LGTM - Dataset field added to schema.The addition of the required
datasetfield to theQueryJobCreationSchemaaligns with the PR objectives to introduce dataset awareness. Note that this is a breaking change asdatasetis now required.Consider adding validation constraints if dataset names have specific formatting requirements.
components/log-viewer-webui/server/src/fastify-v2/routes/api/search/index.ts (1)
63-63: LGTM - Dataset parameter properly integrated.The
datasetparameter is correctly extracted from the request body and passed through to the job managers. The implementation follows the established pattern for other parameters.Also applies to: 73-73
components/log-viewer-webui/client/src/api/search.ts (1)
18-18: LGTM - Client type updated to match server schema.The
datasetproperty addition maintains consistency between client and server type definitions. Good that the TODO comments acknowledge the temporary nature of this duplication until types are moved to a shared location.components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/index.module.css (1)
1-13: LGTM - Well-structured CSS for dataset selector.The CSS module properly styles the dataset selector component:
- Flex container provides appropriate layout
- Fixed width ensures consistent UI sizing
- Border radius removal on the left side suggests proper integration with adjacent controls
- Correct use of
:global()to target Ant Design component classescomponents/log-viewer-webui/client/src/typings/query.ts (1)
52-52: LGTM - Dataset parameter added to extract job parameters.The
datasetproperty addition toExtractJobSearchParamsis consistent with the typebox schema pattern and supports dataset-aware extract operations. The required nature aligns with the broader dataset integration approach.components/log-viewer-webui/client/src/ui/QueryStatus.tsx (1)
68-79: LGTM! Clean API refactoring to object parameters.The refactoring from positional arguments to a named object parameter improves code readability and maintainability. The
datasetfield is properly extracted from the parsed URL parameters and included in the API call, aligning well with the dataset-awareness implementation across the codebase.components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/index.tsx (2)
1-9: LGTM! Clean import organization.The import changes properly bring in the dataset-related components and configuration constants needed for the conditional rendering logic.
30-30: LGTM! Appropriate conditional rendering logic.The conditional rendering of the
Datasetcomponent only when the storage engine isCLP_Smakes perfect sense, as datasets are only relevant for that specific storage engine configuration.components/log-viewer-webui/client/src/api/sql.ts (1)
10-12: LGTM! Clean and well-typed SQL query function.The generic
querySqlfunction provides good type safety with TypeScript generics and follows a clean, straightforward implementation pattern for API calls. The centralization of SQL queries into a dedicated API module is excellent for maintainability.components/clp-package-utils/clp_package_utils/scripts/compress.py (2)
80-81: LGTM! Consistent dataset argument implementation.The addition of the
--datasetCLI argument with a sensible default and its proper inclusion in the compress command generation follows the consistent pattern being implemented across all package utilities.Also applies to: 138-142
9-12: Verify the import availability.The import statement for
clp_py_utils.clp_configis flagged by static analysis as potentially unavailable. Please ensure this module exists and is properly accessible.#!/bin/bash # Description: Verify the clp_py_utils.clp_config module exists and is importable # Expected: Module should be found and importable # Check if the module file exists fd -t f "clp_config.py" --exec echo "Found clp_config.py at: {}" # Check for any Python files in clp_py_utils that might contain these constants fd -t f "*.py" | xargs grep -l "CLP_DEFAULT_DATASET_NAME\|StorageEngine" | head -5components/log-viewer-webui/client/src/AntdApp.tsx (2)
3-7: LGTM! Standard React Query setup.The React Query imports and
QueryClientinstantiation follow standard practices for integrating React Query into the application.Also applies to: 16-16
25-32: LGTM! Proper provider hierarchy and devtools configuration.The
QueryClientProviderwrapper maintains the existing component hierarchy while adding React Query capabilities. TheReactQueryDevtoolsconfiguration withinitialIsOpen={false}is appropriate for production use.components/clp-package-utils/clp_package_utils/scripts/archive_manager.py (3)
8-11: LGTM! Dataset import and constant usage looks correct.The import of
CLP_DEFAULT_DATASET_NAMEaligns with the broader dataset-awareness enhancement across the codebase. The static analysis import error is likely a false positive due to missing dependencies in the analysis environment.
67-71: LGTM! Dataset argument addition is well-implemented.The
--datasetargument with default value and descriptive help text follows good CLI design practices. The implementation is consistent with similar dataset arguments added across other CLI scripts.
207-207: LGTM! Dataset parameter propagation is correctly implemented.The dataset value is properly passed to the native archive manager subprocess command, ensuring dataset context is maintained throughout the execution chain.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts (1)
1-42: LGTM! Well-structured SQL query module.The implementation demonstrates good practices:
- Proper separation of constants and types
- Enum usage for column names prevents magic strings
- Clear SQL query with ordered results
- Async function with proper error handling delegation to
querySql- Clean data transformation in the return statement
The module integrates well with the broader dataset selection functionality.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx (3)
11-11: LGTM! Proper search state integration.The import of
useSearchStorehook enables access to the dataset context for navigation links.
36-36: LGTM! Correct use of cachedDataset.Using
cachedDatasetinstead ofselectDatasetis the right approach to prevent navigation issues when users change dataset selection before query completion.
50-51: LGTM! Dataset parameter properly added to navigation URL.The dataset parameter is correctly encoded and appended to the query string, ensuring log viewer links maintain proper dataset context.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.tsx (1)
1-36: LGTM! Well-implemented label component.The component follows React best practices:
- Clean functional component structure
- Proper use of Ant Design theme tokens for consistent styling
- CSS modules integration for additional styling
- Good JSDoc documentation
- Appropriate component export
The styling approach ensures consistency with the overall application design system.
components/log-viewer-webui/server/src/routes/query.ts (3)
41-41: LGTM! Dataset parameter properly added to request schema.The dataset field is correctly defined as a required string in the request body schema, ensuring proper validation of incoming requests.
48-48: LGTM! Dataset parameter properly extracted from request.The dataset value is correctly destructured from the request body alongside other parameters.
61-61: LGTM! Dataset parameter correctly passed to job submission.The dataset parameter is properly passed to
submitAndWaitForExtractStreamJob, ensuring dataset context is maintained throughout the extraction workflow.components/clp-package-utils/clp_package_utils/scripts/search.py (3)
10-14: LGTM: Import statement correctly expanded.The import statement properly includes
CLP_DEFAULT_DATASET_NAMEwhich is used as the default value for the new dataset argument.
42-46: LGTM: Dataset argument well-implemented.The
--datasetargument follows good CLI practices with:
- Appropriate default value from configuration
- Clear help text describing its purpose
- Consistent naming convention
116-116: LGTM: Dataset parameter correctly propagated.The dataset value is properly passed to the native search script, maintaining consistency across the CLI chain.
components/clp-package-utils/clp_package_utils/scripts/decompress.py (3)
8-13: LGTM: Import properly expanded.The import statement correctly includes
CLP_DEFAULT_DATASET_NAMEfor use in the argument parser.
215-216: LGTM: Dataset parameter correctly added to extraction command.The dataset arguments are properly appended to the extraction command specifically for the
EXTRACT_JSON_CMDcase, which is the appropriate conditional logic.
277-282: LGTM: Dataset argument consistently implemented.The argument definition follows the same pattern as other scripts with:
- Appropriate type specification
- Default value from configuration
- Clear help text
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsx (2)
76-76: LGTM: Dataset parameter properly included.The dataset parameter is correctly passed to the query submission with appropriate fallback to empty string.
65-73: Fix boolean comparison to follow coding guidelines.The boolean comparison should follow the established pattern:
false == <expression>rather than!<expression>.- if (CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE) { - if (null !== selectDataset) { + if (CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE) { + if (false == (null === selectDataset)) {⛔ Skipped due to learnings
Learnt from: junhaoliao PR: y-scope/clp#988 File: components/log-viewer-webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx:34-37 Timestamp: 2025-06-09T17:48:56.024Z Learning: In the y-scope/clp project, prefer `false == <expression>` rather than `!<expression>` for boolean expressions in TypeScript/JavaScript files, as specified in the coding guidelines.components/clp-package-utils/clp_package_utils/scripts/native/archive_manager.py (2)
100-104: LGTM: Dataset argument consistently implemented.The argument definition follows the established pattern across all CLI scripts with appropriate default value and help text.
204-228: LGTM: Dataset parameter properly propagated.The dataset parameter is correctly passed to both
_find_archivesand_delete_archivesfunctions, ensuring dataset-aware operations throughout the archive management workflow.components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/index.tsx (4)
35-46: LGTM: Proper dataset initialization logic.The effect correctly initializes the dataset selection to the first available dataset only when no dataset is currently selected, preventing unwanted overrides.
48-57: LGTM: Appropriate error handling.The error effect properly displays user-friendly error messages when dataset fetching fails, which is important since querying is disabled without datasets.
61-75: LGTM: Proper handling of empty dataset response.The logic correctly handles the case where no datasets are available by:
- Showing a helpful warning message to users
- Resetting dataset state to null to disable query submission
- Using appropriate dependency array
81-96: LGTM: Well-structured component render.The component structure is clean with:
- Proper CSS module usage
- Appropriate Ant Design Select configuration
- Loading state handling
- Search functionality enabled
components/log-viewer-webui/server/src/plugins/DbManager.ts (2)
101-120: LGTM! Clean dataset parameter integration.The method signature and type definition correctly include the new
datasetparameter, maintaining consistency with the broader dataset-awareness changes across the system.
129-135: Correct conditional dataset usage for JSON extraction jobs.The implementation correctly adds the
datasetfield to the job configuration only forEXTRACT_JSONjob types, which aligns with the requirements where JSON extraction jobs need dataset context while IR extraction jobs operate differently.components/clp-package-utils/clp_package_utils/scripts/native/compress.py (3)
13-17: LGTM! Proper import of dataset constant.The import correctly includes
CLP_DEFAULT_DATASET_NAMEwhich provides a consistent default value across the system.
199-203: Well-implemented dataset CLI argument.The argument is properly configured with a sensible default value and clear help text describing its purpose as "the name of the log category."
147-147: Correct dataset propagation in configuration objects.The
datasetparameter is correctly included in bothFsInputConfigandS3InputConfigobjects, ensuring dataset awareness regardless of input type.Also applies to: 162-162
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx (1)
41-49: Good dataset validation for CLP_S storage engine.The conditional check correctly validates that a dataset is selected when using CLP_S storage engine, with appropriate error logging and early return to prevent submission without required dataset information.
components/clp-package-utils/clp_package_utils/scripts/native/decompress.py (2)
44-61: Good dataset-aware table prefix logic.The function correctly handles dataset-specific table prefixes for CLP_S storage engine, enabling proper dataset-scoped queries while maintaining backward compatibility for other storage engines.
315-320: Well-implemented dataset CLI argument for JSON extraction.The argument follows the established pattern with proper default value and descriptive help text, maintaining consistency across the CLI tools.
components/log-viewer-webui/client/src/api/query.ts (2)
12-18: Well-defined interface for function parameters.The new
submitExtractStreamJobPropsinterface provides clear typing for the refactored function parameters, improving code maintainability and type safety.
32-49: Clean refactoring to object parameter pattern.The function signature refactoring from multiple individual parameters to a single object parameter improves maintainability and extensibility. The
datasetparameter is correctly included in the request payload, aligning with the backend dataset-awareness requirements.components/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsx (3)
19-19: Well-designed dataset state separation.The addition of separate
cachedDatasetandselectDatasetstates with appropriate default values is a thoughtful design choice that prevents UI selector changes from affecting already-submitted queries.Also applies to: 26-26
38-43: Clear documentation of dataset state purpose.The comments effectively explain the rationale for having separate cached and selected dataset states, making the codebase more maintainable.
Also applies to: 75-78
98-98: Consistent implementation of dataset state updaters.The updater functions follow the established pattern in the file and properly maintain type safety with the Zustand store.
Also applies to: 105-105, 116-118, 137-139
components/clp-package-utils/clp_package_utils/scripts/native/search.py (4)
36-59: Consistent dataset parameter integration.The dataset parameter is properly integrated into the
SearchJobConfigand function signature. The implementation maintains consistency with the existing codebase patterns.
119-130: Proper parameter propagation through async functions.The dataset parameter is correctly propagated through the async function calls, maintaining the data flow integrity from CLI input to job configuration.
Also applies to: 192-218
243-247: Well-implemented CLI argument for dataset.The
--datasetargument is properly configured with a sensible default value and appropriate help text, following CLI best practices.
13-17: ```shell
#!/bin/bashSearch for any occurrences of the clp_py_utils module in the repository
echo "Finding directories named clp_py_utils..."
find . -type d -name "clp_py_utils"echo "Finding files under clp_py_utils..."
find . -type f | grep "clp_py_utils"echo "Locating any clp_config.py files..."
find . -type f -name "clp_config.py"</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
...iewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.module.css
Show resolved
Hide resolved
...s/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsx
Show resolved
Hide resolved
...-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/native/decompress.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
components/log-viewer-webui/client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (34)
components/clp-package-utils/clp_package_utils/scripts/archive_manager.py(3 hunks)components/clp-package-utils/clp_package_utils/scripts/compress.py(3 hunks)components/clp-package-utils/clp_package_utils/scripts/decompress.py(3 hunks)components/clp-package-utils/clp_package_utils/scripts/native/archive_manager.py(4 hunks)components/clp-package-utils/clp_package_utils/scripts/native/compress.py(4 hunks)components/clp-package-utils/clp_package_utils/scripts/native/decompress.py(6 hunks)components/clp-package-utils/clp_package_utils/scripts/native/search.py(10 hunks)components/clp-package-utils/clp_package_utils/scripts/search.py(3 hunks)components/log-viewer-webui/client/package.json(1 hunks)components/log-viewer-webui/client/src/AntdApp.tsx(2 hunks)components/log-viewer-webui/client/src/api/query.ts(1 hunks)components/log-viewer-webui/client/src/api/search.ts(1 hunks)components/log-viewer-webui/client/src/api/sql.ts(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/sqlConfig.ts(0 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.module.css(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/index.module.css(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton.tsx(0 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/index.tsx(2 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx(3 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsx(3 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsx(6 hunks)components/log-viewer-webui/client/src/typings/query.ts(1 hunks)components/log-viewer-webui/client/src/ui/QueryStatus.tsx(2 hunks)components/log-viewer-webui/server/src/fastify-v2/routes/api/search/index.ts(2 hunks)components/log-viewer-webui/server/src/fastify-v2/schemas/search.ts(1 hunks)components/log-viewer-webui/server/src/plugins/DbManager.ts(2 hunks)components/log-viewer-webui/server/src/routes/query.ts(2 hunks)
💤 Files with no reviewable changes (2)
- components/log-viewer-webui/client/src/pages/IngestPage/sqlConfig.ts
- components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton.tsx
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/typings/query.tscomponents/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsxcomponents/log-viewer-webui/client/src/api/search.tscomponents/log-viewer-webui/client/src/AntdApp.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/index.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/index.tsxcomponents/log-viewer-webui/client/src/api/sql.tscomponents/log-viewer-webui/server/src/fastify-v2/schemas/search.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsxcomponents/log-viewer-webui/server/src/routes/query.tscomponents/log-viewer-webui/client/src/ui/QueryStatus.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.tscomponents/log-viewer-webui/server/src/fastify-v2/routes/api/search/index.tscomponents/log-viewer-webui/server/src/plugins/DbManager.tscomponents/log-viewer-webui/client/src/api/query.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsx
🪛 Pylint (3.3.7)
components/clp-package-utils/clp_package_utils/scripts/decompress.py
[error] 8-13: Unable to import 'clp_py_utils.clp_config'
(E0401)
components/clp-package-utils/clp_package_utils/scripts/archive_manager.py
[error] 8-11: Unable to import 'clp_py_utils.clp_config'
(E0401)
components/clp-package-utils/clp_package_utils/scripts/native/compress.py
[error] 13-17: Unable to import 'clp_py_utils.clp_config'
(E0401)
components/clp-package-utils/clp_package_utils/scripts/search.py
[error] 10-14: Unable to import 'clp_py_utils.clp_config'
(E0401)
components/clp-package-utils/clp_package_utils/scripts/compress.py
[error] 9-12: Unable to import 'clp_py_utils.clp_config'
(E0401)
components/clp-package-utils/clp_package_utils/scripts/native/search.py
[error] 13-17: Unable to import 'clp_py_utils.clp_config'
(E0401)
🔇 Additional comments (49)
components/log-viewer-webui/client/package.json (1)
22-23: LGTM! React Query dependencies properly added.The addition of React Query and its devtools supports the new dataset selector functionality with proper state management and caching capabilities.
components/log-viewer-webui/server/src/fastify-v2/schemas/search.ts (1)
13-13: Dataset field properly added to query schema.The required dataset string field correctly extends the query job creation schema to support dataset-aware operations.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.module.css (1)
1-9: Well-structured CSS for dataset label component.The flexbox layout with appropriate dimensions and border radius styling correctly supports the dataset selector UI component.
components/log-viewer-webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (1)
9-9: Import path correctly updated for refactored API module.The change from relative to absolute import path properly reflects the centralization of SQL query functionality into the dedicated API module.
components/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsx (1)
10-10: Consistent import path update for API refactoring.The import path change aligns with the centralization of SQL query functionality and maintains consistency across the codebase.
components/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsx (1)
9-9: LGTM! Import path refactoring looks good.The import path change properly reflects the relocation of
querySqlfrom a page-specific module to a centralized API module, improving code organization.components/log-viewer-webui/server/src/fastify-v2/routes/api/search/index.ts (1)
63-63: Dataset parameter integration looks correct.The dataset parameter is properly extracted from the request body and passed through to the job submission functions. Ensure that the
QueryJobCreationSchemaincludes proper validation for the dataset field.Also applies to: 73-73
components/log-viewer-webui/client/src/api/search.ts (1)
18-18: Dataset field addition is correct.The required
datasetfield properly extends the schema to support dataset specification in query submissions, consistent with the broader dataset awareness changes.components/log-viewer-webui/client/src/typings/query.ts (1)
52-52: Dataset parameter addition to extract job schema is correct.The
datasetfield is properly added usingType.String(), extending the URL search parameters for extract jobs to support dataset specification.components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/index.tsx (2)
1-5: LGTM! Clean integration of dataset functionality.The imports properly bring in the necessary configuration constants and Dataset component to support conditional dataset selection based on storage engine type.
30-30: Dataset selector correctly conditional on storage engine.The conditional rendering ensures the Dataset component only appears when using CLP_S storage engine, which aligns with the PR objectives of adding dataset-aware functionality.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx (2)
11-11: Proper integration of search store for dataset context.The useSearchStore hook is correctly imported and used to retrieve the cachedDataset, ensuring consistent dataset context across navigation.
Also applies to: 36-36
50-51: URL parameter construction is secure and correct.The dataset parameter is properly encoded using encodeURIComponent to prevent URL injection issues and ensure proper parameter transmission.
components/log-viewer-webui/client/src/ui/QueryStatus.tsx (1)
68-79: Excellent refactoring to named parameters.The change from positional arguments to an object with named properties significantly improves code readability and maintainability. The dataset parameter is properly integrated from the parsed URL parameters.
components/log-viewer-webui/client/src/AntdApp.tsx (1)
3-7: Clean React Query integration following best practices.The QueryClient setup and provider hierarchy is correctly implemented. The ReactQueryDevtools with
initialIsOpen={false}is appropriate for production builds, and the provider structure properly wraps the existing ConfigProvider.Also applies to: 16-16, 25-32
components/log-viewer-webui/client/src/api/sql.ts (1)
1-14: Well-designed utility function for SQL queries.The function is appropriately generic with TypeScript typing, uses standard axios patterns, and centralizes SQL query functionality as mentioned in previous reviews. The simple implementation allows callers to handle errors as needed.
components/clp-package-utils/clp_package_utils/scripts/archive_manager.py (3)
8-11: Import implementation looks correct.The static analysis error about importing 'clp_py_utils.clp_config' is likely a false positive since this appears to be a valid internal package import based on the project structure.
67-71: LGTM! Dataset argument properly implemented.The dataset argument follows the established CLI pattern with appropriate default value and help text.
207-207: ```shell
#!/bin/bashLocate the native archive_manager script
native_script=$(find components/clp-package-utils -type f -path "*scripts/native/archive_manager.py")
echo "Native script path: $native_script"Verify argparse import and parser setup
grep -n "import argparse" "$native_script" || echo "
⚠️ No argparse import found in native script"
grep -n "argparse.ArgumentParser" "$native_script" || echo "⚠️ No ArgumentParser instantiation found"Check for --dataset argument definition
grep -n "add_argument.*--dataset" "$native_script" || echo "
⚠️ No --dataset add_argument in native script"Show nearby context for any dataset handling
grep -n -C3 "dataset" "$native_script" || echo "
⚠️ No dataset references found in native script"</details> <details> <summary>components/log-viewer-webui/server/src/routes/query.ts (1)</summary> `48-48`: **LGTM! Dataset parameter properly integrated.** The dataset parameter is correctly extracted from the request body and passed to the database manager method. The implementation follows the established pattern. Also applies to: 61-61 </details> <details> <summary>components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.tsx (1)</summary> `16-33`: **LGTM! Well-structured React component.** The component properly uses Ant Design theming and follows React best practices. The dynamic styling with theme tokens is implemented correctly. </details> <details> <summary>components/clp-package-utils/clp_package_utils/scripts/search.py (3)</summary> `10-14`: **Import implementation consistent and correct.** The import follows the same pattern as other scripts in this PR. The static analysis error is likely a false positive. --- `42-46`: **LGTM! Consistent dataset argument implementation.** The dataset argument implementation is identical to the pattern used in `archive_manager.py`, ensuring consistency across CLI scripts. --- `116-116`: **Dataset parameter properly propagated.** The dataset argument is correctly passed to the native search module, maintaining the parameter flow through the command chain. </details> <details> <summary>components/clp-package-utils/clp_package_utils/scripts/native/compress.py (1)</summary> `13-17`: **LGTM: Clean integration of dataset parameter** The implementation correctly adds dataset support throughout the compression workflow: - Proper import of the default dataset constant - Consistent parameter passing to both filesystem and S3 input configurations - Well-documented CLI argument with appropriate default value Also applies to: 147-147, 162-162, 199-203 </details> <details> <summary>components/clp-package-utils/clp_package_utils/scripts/compress.py (1)</summary> `9-12`: **LGTM: Consistent dataset parameter integration** The changes properly integrate dataset support: - Correct import of the default dataset constant - Proper propagation of dataset argument to the subprocess command - Consistent CLI argument definition matching the native script pattern Also applies to: 80-81, 138-142 </details> <details> <summary>components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsx (2)</summary> `8-11`: **LGTM: Clean refactoring to individual selectors** The refactor from destructuring to individual `useSearchStore` selectors improves readability and follows React best practices for fine-grained subscriptions. Also applies to: 26-36 --- `65-77`: **LGTM: Proper dataset validation and state management** The implementation correctly: - Validates dataset selection is required for CLP_S storage engine - Updates cached dataset state before query submission - Provides clear error messaging for missing dataset - Follows the coding guidelines by using `null !== selectDataset` instead of `!selectDataset` - Passes dataset parameter with proper fallback to empty string </details> <details> <summary>components/clp-package-utils/clp_package_utils/scripts/decompress.py (1)</summary> `8-13`: **LGTM: Targeted dataset support for JSON extraction** The implementation correctly adds dataset support specifically for JSON extraction: - Proper import of the default dataset constant - Dataset parameter appropriately added only to the `EXTRACT_JSON` command path - Consistent CLI argument definition with proper default and help text - Selective application makes sense as JSON extraction likely requires dataset context Also applies to: 215-216, 277-282 </details> <details> <summary>components/log-viewer-webui/server/src/plugins/DbManager.ts (1)</summary> `101-101`: **LGTM: Consistent backend dataset parameter integration** The implementation properly adds dataset support to the extraction job workflow: - Method signature correctly updated with required dataset parameter - JSDoc documentation properly reflects the new parameter - Dataset parameter appropriately added to the `EXTRACT_JSON` job configuration - Type definitions are consistent and match the implementation - Aligns well with the frontend and CLI script changes Also applies to: 109-109, 115-115, 131-131 </details> <details> <summary>components/clp-package-utils/clp_package_utils/scripts/native/archive_manager.py (2)</summary> `100-104`: **LGTM: Dataset argument added correctly.** The `--dataset` argument is properly integrated with appropriate default value and help text. The parameter flows correctly to both find and delete operations. --- `204-204`: **LGTM: Dataset parameter properly propagated.** The dataset parameter is correctly passed through to the `_find_archives` and `_delete_archives` functions for both deletion subcommands, ensuring consistent dataset-aware operations. Also applies to: 216-216, 228-228 </details> <details> <summary>components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx (3)</summary> `86-87`: **Fix coding guideline violation in disabled prop.** ```diff - disabled={isQueryStringEmpty || false === isSomeDataset || + disabled={isQueryStringEmpty || isDatasetRequiredButNotSelected || searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING}Likely an incorrect or invalid review comment.
72-77: Fix coding guideline violation and update logic.let tooltipTitle = ""; - if (false === isSomeDataset) { + if (isDatasetRequiredButNotSelected) { tooltipTitle = "Some data must be ingested to enable search"; } else if (isQueryStringEmpty) { tooltipTitle = "Enter query to search"; }Likely an incorrect or invalid review comment.
67-71: Fix the dataset validation logic and adhere to coding guidelines.The current logic has issues:
- Coding guideline violation: Use
!isSomeDatasetinstead offalse === isSomeDatasetas per the coding guidelines.- Logic error: The variable name
isSomeDatasetis misleading. The logic should validate if a dataset is required (for CLP_S) AND not selected.- // Submit button must be disabled if there are no datasets since clp-s requires dataset option - // for queries. - const isSomeDataset = (null !== selectDataset) && - (CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE); + // Submit button must be disabled if dataset is required but not selected + const isDatasetRequiredButNotSelected = (CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE) && + (null === selectDataset);Likely an incorrect or invalid review comment.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/index.tsx (1)
21-99: LGTM: Well-implemented dataset selector component.The component demonstrates good practices:
- Proper use of React Query for data fetching with appropriate stale time
- Comprehensive error and empty state handling with user-friendly messages
- Correct dependency arrays in useEffect hooks
- Sensible auto-selection logic for the first available dataset
- Clean integration with the search store and Ant Design components
The implementation properly handles the dataset selection requirement for CLP_S storage engine.
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsx (1)
19-19: LGTM: Clean dataset state management implementation.The addition of
cachedDatasetandselectDatasetproperties is well-designed:
- Clear separation between UI selection state (
selectDataset) and submitted query state (cachedDataset)- Proper default values and type annotations
- Consistent implementation across interface, default state, and store methods
- Good documentation explaining the purpose of each dataset property
This design prevents issues where UI changes could affect ongoing queries or log viewer links.
Also applies to: 26-26, 39-44, 75-79, 98-98, 105-105, 116-118, 137-139
components/log-viewer-webui/client/src/api/query.ts (1)
12-49: LGTM: Clean API refactoring with dataset support.The refactoring improves the API design:
- Using object parameter pattern makes the function more maintainable and readable
- Proper TypeScript interface definition ensures type safety
- Dataset parameter correctly integrated into the request payload
- JSDoc documentation updated appropriately
The change aligns well with the broader dataset support being added across the system.
components/clp-package-utils/clp_package_utils/scripts/native/decompress.py (5)
17-17: LGTM: Import addition is appropriate.The StorageEngine import is correctly added to support the new conditional table prefix logic.
44-46: LGTM: Function signature enhancement for dataset support.The addition of
storage_engineanddatasetparameters toget_orig_file_idenables proper dataset-aware table naming, which aligns with the PR's objective of adding dataset support.
59-61: LGTM: Conditional table prefix logic is well-implemented.The conditional logic correctly appends the dataset name to the table prefix when using CLP_S storage engine, ensuring proper dataset isolation at the database level.
156-156: LGTM: Correct use of parsed dataset argument.The change from hardcoded
CLP_DEFAULT_DATASET_NAMEtoparsed_args.datasetproperly utilises the user-specified dataset parameter.
315-320: LGTM: CLI argument addition follows established patterns.The
--datasetargument is properly configured with appropriate default value and help text, consistent with other CLI enhancements in the PR.components/clp-package-utils/clp_package_utils/scripts/native/search.py (6)
13-17: LGTM: Import update supports dataset functionality.The addition of
CLP_DEFAULT_DATASET_NAMEimport is necessary for the new CLI default value.
39-39: LGTM: Proper dataset parameter integration.The dataset parameter is correctly added to the function signature and properly passed to
SearchJobConfig, ensuring dataset-aware search operations.Also applies to: 51-51
122-122: LGTM: Consistent parameter propagation.The dataset parameter is correctly added to
do_search_without_aggregationand properly passed through the function call chain.Also applies to: 157-157
195-195: LGTM: Complete dataset parameter threading.The dataset parameter is consistently added to
do_searchand properly propagated through all function calls, maintaining the parameter flow throughout the search workflow.Also applies to: 210-210, 224-224
243-247: LGTM: CLI argument follows established conventions.The
--datasetargument is properly configured with appropriate default value and descriptive help text, consistent with the dataset CLI enhancements across the codebase.
305-305: LGTM: Proper use of parsed dataset argument.The parsed dataset argument is correctly passed to the search function, completing the CLI-to-implementation parameter flow.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/index.module.css
Show resolved
Hide resolved
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/index.module.css
Show resolved
Hide resolved
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts
Show resolved
Hide resolved
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/sql.ts
Show resolved
Hide resolved
...s/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.tsx
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/native/decompress.py
Outdated
Show resolved
Hide resolved
davemarco
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.module.css (1)
7-8: Unify units for zero border-radius and verify left corners
The border-radius zero values currently use0px. It's more consistent to use0without units. Also confirm whether the left-side corners need explicit radii (e.g.,border-top-left-radius/border-bottom-left-radius) to match the surrounding UI.
Apply this diff:- border-top-right-radius: 0px; - border-bottom-right-radius: 0px; + border-top-right-radius: 0; + border-bottom-right-radius: 0;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.module.css(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx (2)
67-70: Dataset validation logic has been properly fixed.The previous issue with dataset validation has been resolved. The current implementation correctly uses
isNoDatasetsAndClpSto only disable the button when both conditions are true: no dataset is selected AND the storage engine is CLP_S. For non-CLP_S storage engines, the button will remain enabled regardless of dataset selection status, which is the correct behaviour.
52-52: Let’s pull in all uses ofdatasetin the DbManager to see how it’s applied:#!/bin/bash # Search for `dataset` parameter usage with surrounding context rg -n "dataset" -C10 components/log-viewer-webui/server/src/plugins/DbManager.ts
| min-width: 80px; | ||
| height: 100%; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using CSS variables for spacing
The min-width: 80px; and height: 100%; values are hard-coded. Leveraging CSS custom properties (e.g. var(--dataset-label-min-width)) or theme tokens will improve maintainability and align with the design system.
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/Dataset/DatasetLabel/index.module.css
at lines 2 to 3, replace the hard-coded values for min-width and height with CSS
variables. Define and use custom properties like var(--dataset-label-min-width)
for min-width and a suitable variable for height to improve maintainability and
consistency with the design system.
| if (CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE) { | ||
| if (null !== selectDataset) { | ||
| updateCachedDataset(selectDataset); | ||
| } else { | ||
| console.error("Cannot submit a clp-s query without a dataset selection."); | ||
|
|
||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider enhancing user feedback for dataset validation errors.
While the dataset validation logic is correct, the error on line 45 is only logged to the console and not displayed to the user. Although the button should be disabled when no dataset is selected for CLP_S, this error handling could be improved for better user experience in edge cases.
Consider adding user-visible error feedback:
} else {
- console.error("Cannot submit a clp-s query without a dataset selection.");
+ console.error("Cannot submit a clp-s query without a dataset selection.");
+ // Consider adding a toast notification or error state to inform the user
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE) { | |
| if (null !== selectDataset) { | |
| updateCachedDataset(selectDataset); | |
| } else { | |
| console.error("Cannot submit a clp-s query without a dataset selection."); | |
| return; | |
| } | |
| } | |
| if (CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE) { | |
| if (null !== selectDataset) { | |
| updateCachedDataset(selectDataset); | |
| } else { | |
| console.error("Cannot submit a clp-s query without a dataset selection."); | |
| // Consider adding a toast notification or setting an error state here | |
| return; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx
around lines 41 to 49, the dataset validation error is only logged to the
console and not shown to the user. To improve user feedback, add a user-visible
error message or notification when no dataset is selected for CLP_S, ensuring
the user understands why the submission is blocked even in edge cases.
components/clp-package-utils/clp_package_utils/scripts/archive_manager.py
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/native/compress.py
Show resolved
Hide resolved
| import msgpack | ||
| import pymongo | ||
| from clp_py_utils.clp_config import Database, ResultsCache | ||
| from clp_py_utils.clp_config import ( |
There was a problem hiding this comment.
For archive manager and decompress, I feel we should validate the dataset argument provided by the users. If they give a dataset that doesn't exist in the database, we should return some error/warning instead of silently not returning results.
There was a problem hiding this comment.
I think in the query scheduler, if user provides a dataset name that is not currently registered, then query scheduler would log an error. But I don't think that error shows up on the user interface yet.
There was a problem hiding this comment.
Yep I am aware that query scheduler will log an error, but archive_manager.sh and decompress.sh doesn't go through the query scheduler iirc. I think we should:
- For archive manager and decompress, returns an error message to user in the cmdline if the dataset doesn't exist
- For search.sh, it would be great if we can forward query scheduler's error to user, showing them that the dataset doesn't exists.
- For webui, I assume we will only be able to choose from the available datasets (i.e. user can't specify any random dataset)? if so then I guess it's ok if we don't do anything special.
There was a problem hiding this comment.
for webui - you can only choose from available datasets. user cannot submit query if there is no dataset.
| }; | ||
| } else if (QUERY_JOB_TYPE.EXTRACT_JSON === jobType) { | ||
| jobConfig = { | ||
| dataset: dataset, |
There was a problem hiding this comment.
Can the dataset be set to null here when engine type is CLP?
Ideally, it would be great if dataset is set to Null for clp , and we should be able to have such support in python part of the package.
...-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/archive_manager.py
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/archive_manager.py
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/native/decompress.py
Outdated
Show resolved
Hide resolved
Bill-hbrhbr
force-pushed
the
dataset-interface
branch
from
1759ea9 to
18ebff6
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (7)
components/log-viewer-webui/package-lock.jsonis excluded by!**/package-lock.jsoncomponents/webui/client/package-lock.jsonis excluded by!**/package-lock.jsoncomponents/webui/client/public/clp-logo.pngis excluded by!**/*.pngcomponents/webui/client/public/favicon.svgis excluded by!**/*.svgcomponents/webui/linter/package-lock.jsonis excluded by!**/package-lock.jsoncomponents/webui/package-lock.jsonis excluded by!**/package-lock.jsoncomponents/webui/server/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (107)
components/clp-package-utils/clp_package_utils/general.py(1 hunks)components/clp-package-utils/clp_package_utils/scripts/start_clp.py(6 hunks)components/clp-package-utils/clp_package_utils/scripts/stop_clp.py(0 hunks)components/clp-py-utils/clp_py_utils/clp_config.py(2 hunks)components/clp-py-utils/clp_py_utils/s3_utils.py(2 hunks)components/log-viewer-webui/.gitignore(0 hunks)components/log-viewer-webui/README.md(0 hunks)components/log-viewer-webui/client/README.md(0 hunks)components/log-viewer-webui/client/public/settings.json(0 hunks)components/log-viewer-webui/client/src/App.tsx(0 hunks)components/log-viewer-webui/package.json(0 hunks)components/package-template/src/etc/clp-config.yml(1 hunks)components/webui/.gitignore(1 hunks)components/webui/.meteor/.finished-upgraders(0 hunks)components/webui/.meteor/.gitignore(0 hunks)components/webui/.meteor/.id(0 hunks)components/webui/.meteor/packages(0 hunks)components/webui/.meteor/platforms(0 hunks)components/webui/.meteor/release(0 hunks)components/webui/.meteor/versions(0 hunks)components/webui/.meteorignore(0 hunks)components/webui/README.md(1 hunks)components/webui/client/index.html(1 hunks)components/webui/client/main.css(0 hunks)components/webui/client/main.html(0 hunks)components/webui/client/main.jsx(0 hunks)components/webui/client/package.json(2 hunks)components/webui/client/public/settings.json(1 hunks)components/webui/client/src/App.tsx(1 hunks)components/webui/client/src/api/query.ts(1 hunks)components/webui/client/src/api/search.ts(1 hunks)components/webui/client/src/api/sql.ts(1 hunks)components/webui/client/src/main.tsx(1 hunks)components/webui/client/src/pages/IngestPage/Details/index.tsx(1 hunks)components/webui/client/src/pages/IngestPage/Details/sql.ts(3 hunks)components/webui/client/src/pages/IngestPage/Jobs/index.tsx(1 hunks)components/webui/client/src/pages/IngestPage/Jobs/sql.ts(2 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx(1 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts(2 hunks)components/webui/client/src/pages/IngestPage/sqlConfig.ts(0 hunks)components/webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/SearchButton/index.module.css(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/SearchButton/index.tsx(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/index.tsx(2 hunks)components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx(3 hunks)components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsx(3 hunks)components/webui/client/src/pages/SearchPage/SearchState/index.tsx(6 hunks)components/webui/client/src/settings.ts(1 hunks)components/webui/client/src/typings/query.ts(1 hunks)components/webui/client/src/ui/QueryStatus.tsx(2 hunks)components/webui/client/vite.config.ts(1 hunks)components/webui/imports/api/constants.js(0 hunks)components/webui/imports/api/ingestion/collections.js(0 hunks)components/webui/imports/api/ingestion/constants.js(0 hunks)components/webui/imports/api/ingestion/server/CompressionDbManager.js(0 hunks)components/webui/imports/api/ingestion/server/StatsDbManager.js(0 hunks)components/webui/imports/api/ingestion/server/publications.js(0 hunks)components/webui/imports/api/ingestion/types.js(0 hunks)components/webui/imports/api/search/SearchJobCollectionsManager.js(0 hunks)components/webui/imports/api/search/collections.js(0 hunks)components/webui/imports/api/search/constants.js(0 hunks)components/webui/imports/api/search/server/QueryJobsDbManager.js(0 hunks)components/webui/imports/api/search/server/collections.js(0 hunks)components/webui/imports/api/search/server/methods.js(0 hunks)components/webui/imports/api/search/server/publications.js(0 hunks)components/webui/imports/api/search/types.js(0 hunks)components/webui/imports/ui/App.jsx(0 hunks)components/webui/imports/ui/App.scss(0 hunks)components/webui/imports/ui/IngestView/IngestView.jsx(0 hunks)components/webui/imports/ui/IngestView/IngestView.scss(0 hunks)components/webui/imports/ui/IngestView/Panel.jsx(0 hunks)components/webui/imports/ui/IngestView/panels/Details/DetailsRow.jsx(0 hunks)components/webui/imports/ui/IngestView/panels/Details/index.jsx(0 hunks)components/webui/imports/ui/IngestView/panels/IngestionJobs/IngestionJobRow.jsx(0 hunks)components/webui/imports/ui/IngestView/panels/IngestionJobs/IngestionJobs.scss(0 hunks)components/webui/imports/ui/IngestView/panels/IngestionJobs/PlaceholderText.jsx(0 hunks)components/webui/imports/ui/IngestView/panels/IngestionJobs/PlaceholderText.scss(0 hunks)components/webui/imports/ui/IngestView/panels/IngestionJobs/index.jsx(0 hunks)components/webui/imports/ui/IngestView/panels/SpaceSavings.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsCaseSensitivity/SearchControlsCaseSensitivityCheck.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsCaseSensitivity/index.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsFilterDrawer.scss(0 hunks)components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsFilterLabel.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsTimeRangeInput/SearchControlsDatePicker.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsTimeRangeInput/SearchControlsTimeRangeInput.scss(0 hunks)components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsTimeRangeInput/index.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/index.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchControls/index.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchResults/SearchResultsHeader/SearchResultsHeader.scss(0 hunks)components/webui/imports/ui/SearchView/SearchResults/SearchResultsHeader/index.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchResults/SearchResultsTable/SearchResultsLoadSensor.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchResults/SearchResultsTable/SearchResultsLoadSensor.scss(0 hunks)components/webui/imports/ui/SearchView/SearchResults/SearchResultsTable/SearchResultsTable.scss(0 hunks)components/webui/imports/ui/SearchView/SearchResults/SearchResultsTable/index.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchResults/SearchResultsTimeline/SearchResultsTimeline.scss(0 hunks)components/webui/imports/ui/SearchView/SearchResults/SearchResultsTimeline/index.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchResults/index.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchStatus.jsx(0 hunks)components/webui/imports/ui/SearchView/SearchStatus.scss(0 hunks)components/webui/imports/ui/SearchView/SearchView.jsx(0 hunks)components/webui/imports/ui/Sidebar/Sidebar.jsx(0 hunks)components/webui/imports/ui/Sidebar/Sidebar.scss(0 hunks)components/webui/imports/ui/Sidebar/SidebarButton.jsx(0 hunks)components/webui/imports/ui/bootstrap-customized.scss(0 hunks)components/webui/imports/ui/constants.js(0 hunks)components/webui/imports/utils/DbManager.js(0 hunks)components/webui/imports/utils/datetime.js(0 hunks)
⛔ Files not processed due to max files limit (22)
- components/webui/imports/utils/logger.js
- components/webui/imports/utils/misc.js
- components/webui/imports/utils/mongo.js
- components/webui/launcher.js
- components/webui/linter/package.json
- components/webui/package.json
- components/webui/server/main.js
- components/webui/server/package.json
- components/webui/server/settings.json
- components/webui/server/src/fastify-v2/routes/api/search/index.ts
- components/webui/server/src/fastify-v2/schemas/search.ts
- components/webui/server/src/plugins/DbManager.ts
- components/webui/server/src/routes/query.ts
- components/webui/settings.json
- components/webui/tests/main.js
- components/webui/tests/misc.js
- docs/src/dev-guide/components-log-viewer-webui.md
- docs/src/dev-guide/components-webui.md
- docs/src/dev-guide/index.md
- taskfile.yaml
- taskfiles/deps/main.yaml
- taskfiles/lint.yaml
💤 Files with no reviewable changes (75)
- components/log-viewer-webui/.gitignore
- components/webui/imports/ui/IngestView/panels/IngestionJobs/PlaceholderText.scss
- components/webui/client/main.css
- components/webui/.meteor/release
- components/webui/imports/ui/IngestView/panels/IngestionJobs/IngestionJobs.scss
- components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsTimeRangeInput/SearchControlsTimeRangeInput.scss
- components/webui/.meteor/platforms
- components/log-viewer-webui/client/public/settings.json
- components/webui/.meteor/.id
- components/webui/imports/ui/SearchView/SearchResults/SearchResultsTimeline/SearchResultsTimeline.scss
- components/webui/.meteorignore
- components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsFilterDrawer.scss
- components/webui/.meteor/.gitignore
- components/log-viewer-webui/client/README.md
- components/webui/imports/ui/SearchView/SearchResults/SearchResultsHeader/SearchResultsHeader.scss
- components/webui/imports/api/search/server/collections.js
- components/log-viewer-webui/README.md
- components/webui/.meteor/.finished-upgraders
- components/webui/imports/ui/bootstrap-customized.scss
- components/webui/imports/ui/SearchView/SearchResults/SearchResultsTable/SearchResultsLoadSensor.scss
- components/webui/client/main.html
- components/webui/imports/ui/SearchView/SearchStatus.scss
- components/log-viewer-webui/package.json
- components/webui/imports/api/search/collections.js
- components/webui/client/main.jsx
- components/webui/imports/api/constants.js
- components/webui/imports/api/ingestion/types.js
- components/webui/imports/ui/constants.js
- components/webui/.meteor/packages
- components/webui/imports/ui/App.scss
- components/webui/.meteor/versions
- components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsCaseSensitivity/index.jsx
- components/webui/imports/ui/Sidebar/Sidebar.scss
- components/webui/imports/ui/SearchView/SearchResults/SearchResultsTable/SearchResultsTable.scss
- components/webui/imports/api/ingestion/collections.js
- components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsCaseSensitivity/SearchControlsCaseSensitivityCheck.jsx
- components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsFilterLabel.jsx
- components/webui/imports/ui/IngestView/IngestView.jsx
- components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsTimeRangeInput/SearchControlsDatePicker.jsx
- components/log-viewer-webui/client/src/App.tsx
- components/webui/imports/api/ingestion/constants.js
- components/webui/imports/ui/SearchView/SearchStatus.jsx
- components/webui/imports/ui/App.jsx
- components/webui/imports/ui/IngestView/panels/IngestionJobs/IngestionJobRow.jsx
- components/webui/imports/ui/IngestView/panels/Details/index.jsx
- components/webui/imports/ui/IngestView/panels/IngestionJobs/index.jsx
- components/webui/imports/ui/IngestView/Panel.jsx
- components/webui/imports/ui/Sidebar/SidebarButton.jsx
- components/webui/imports/ui/IngestView/IngestView.scss
- components/clp-package-utils/clp_package_utils/scripts/stop_clp.py
- components/webui/imports/ui/IngestView/panels/SpaceSavings.jsx
- components/webui/imports/api/ingestion/server/CompressionDbManager.js
- components/webui/imports/api/search/types.js
- components/webui/imports/ui/IngestView/panels/IngestionJobs/PlaceholderText.jsx
- components/webui/client/src/pages/IngestPage/sqlConfig.ts
- components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/index.jsx
- components/webui/imports/ui/Sidebar/Sidebar.jsx
- components/webui/imports/ui/SearchView/SearchResults/SearchResultsTable/SearchResultsLoadSensor.jsx
- components/webui/imports/ui/IngestView/panels/Details/DetailsRow.jsx
- components/webui/imports/ui/SearchView/SearchResults/SearchResultsHeader/index.jsx
- components/webui/imports/ui/SearchView/SearchControls/SearchControlsFilterDrawer/SearchControlsTimeRangeInput/index.jsx
- components/webui/imports/api/search/SearchJobCollectionsManager.js
- components/webui/imports/api/ingestion/server/StatsDbManager.js
- components/webui/imports/ui/SearchView/SearchControls/index.jsx
- components/webui/imports/ui/SearchView/SearchView.jsx
- components/webui/imports/utils/DbManager.js
- components/webui/imports/api/search/server/publications.js
- components/webui/imports/api/search/server/QueryJobsDbManager.js
- components/webui/imports/ui/SearchView/SearchResults/SearchResultsTimeline/index.jsx
- components/webui/imports/ui/SearchView/SearchResults/index.jsx
- components/webui/imports/api/search/server/methods.js
- components/webui/imports/api/search/constants.js
- components/webui/imports/api/ingestion/server/publications.js
- components/webui/imports/ui/SearchView/SearchResults/SearchResultsTable/index.jsx
- components/webui/imports/utils/datetime.js
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
components/webui/client/src/pages/IngestPage/Jobs/index.tsxcomponents/webui/client/src/pages/IngestPage/Details/index.tsxcomponents/webui/client/vite.config.tscomponents/webui/client/src/api/search.tscomponents/webui/client/src/pages/IngestPage/SpaceSavings/sql.tscomponents/webui/client/src/typings/query.tscomponents/webui/client/src/pages/IngestPage/Jobs/sql.tscomponents/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsxcomponents/webui/client/src/main.tsxcomponents/webui/client/src/pages/SearchPage/SearchControls/index.tsxcomponents/webui/client/src/api/sql.tscomponents/webui/client/src/pages/SearchPage/SearchControls/SearchButton/index.tsxcomponents/webui/client/src/pages/IngestPage/Details/sql.tscomponents/webui/client/src/ui/QueryStatus.tsxcomponents/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsxcomponents/webui/client/src/App.tsxcomponents/webui/client/src/settings.tscomponents/webui/client/src/pages/IngestPage/SpaceSavings/index.tsxcomponents/webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsxcomponents/webui/client/src/pages/SearchPage/SearchState/index.tsxcomponents/webui/client/src/api/query.ts
🧠 Learnings (24)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/README.md (4)
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: quinntaylormitchell
PR: y-scope/clp#968
File: docs/src/user-guide/quick-start/overview.md:53-54
Timestamp: 2025-06-18T20:48:48.990Z
Learning: CLP is designed to run on Linux systems where Python is typically pre-installed, so Python installation links are generally not needed in CLP documentation.
Learnt from: quinntaylormitchell
PR: y-scope/clp#968
File: docs/src/user-guide/quick-start/overview.md:73-109
Timestamp: 2025-06-18T20:39:05.899Z
Learning: The CLP project team prefers to use video content to demonstrate detailed procedural steps (like tarball extraction) rather than including every step in the written documentation, keeping the docs focused on conceptual guidance.
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/del_archives.py:56-65
Timestamp: 2024-11-18T16:49:20.248Z
Learning: When reviewing wrapper scripts in `components/clp-package-utils/clp_package_utils/scripts/`, note that it's preferred to keep error handling simple without adding extra complexity.
components/webui/client/index.html (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/IngestPage/Jobs/index.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/IngestPage/Details/index.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
components/webui/client/vite.config.ts (3)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#1015
File: components/log-viewer-webui/server/src/routes/static.ts:65-70
Timestamp: 2025-06-16T13:05:27.349Z
Learning: In components/log-viewer-webui/server/src/routes/static.ts, when decorateReply is set to true in fastifyStatic configuration, the reply.sendFile() method automatically uses the root directory configured in the static plugin registration, eliminating the need to pass the root directory as a second parameter.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
components/webui/.gitignore (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/api/search.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:90-96
Timestamp: 2024-11-15T16:22:33.635Z
Learning: In the CLP codebase, `table_prefix` is a trusted configuration value and can be safely embedded into SQL queries.
components/webui/client/src/typings/query.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/IngestPage/Jobs/sql.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/package-template/src/etc/clp-config.yml (2)
Learnt from: gibber9809
PR: y-scope/clp#955
File: components/core/src/clp_s/search/sql/CMakeLists.txt:8-26
Timestamp: 2025-06-02T18:22:24.060Z
Learning: In the clp project, ANTLR code generation at build time is being removed by another PR. When reviewing CMake files, be aware that some temporary suboptimal configurations may exist to reduce merge conflicts between concurrent PRs, especially around ANTLR_TARGET calls.
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The webui server logging can be configured via LOG_LEVEL and WEBUI_LOGS_DIR environment variables, with file logging enabled automatically in non-TTY environments when a logs directory is specified.
components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx (6)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.367Z
Learning: React Router v7.4.1 uses a different package structure than previous versions. Components can be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions. Alternatively, DOM-specific components can also be imported using deep imports like `import { Link } from "react-router/dom";`.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
components/webui/client/package.json (5)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
components/webui/client/src/main.tsx (4)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.367Z
Learning: React Router v7.4.1 uses a different package structure than previous versions. Components can be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions. Alternatively, DOM-specific components can also be imported using deep imports like `import { Link } from "react-router/dom";`.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/routes/routes.tsx:1-1
Timestamp: 2025-04-04T15:15:59.432Z
Learning: In React Router v7, `createBrowserRouter` and other router primitives should be imported from `react-router` package, not from `react-router-dom` as in previous versions. This represents a significant change in the library's structure where router primitives are now in the core package.
components/webui/client/src/api/sql.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/IngestPage/Details/sql.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/ui/QueryStatus.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsx (3)
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/App.tsx (4)
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/routes/routes.tsx:1-1
Timestamp: 2025-04-04T15:15:59.432Z
Learning: In React Router v7, `createBrowserRouter` and other router primitives should be imported from `react-router` package, not from `react-router-dom` as in previous versions. This represents a significant change in the library's structure where router primitives are now in the core package.
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.367Z
Learning: React Router v7.4.1 uses a different package structure than previous versions. Components can be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions. Alternatively, DOM-specific components can also be imported using deep imports like `import { Link } from "react-router/dom";`.
components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/clp-package-utils/clp_package_utils/general.py (1)
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The webui server logging can be configured via LOG_LEVEL and WEBUI_LOGS_DIR environment variables, with file logging enabled automatically in non-TTY environments when a logs directory is specified.
components/webui/client/src/api/query.ts (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:35-41
Timestamp: 2024-11-19T19:52:43.429Z
Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.
components/clp-package-utils/clp_package_utils/scripts/start_clp.py (2)
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The webui server logging can be configured via LOG_LEVEL and WEBUI_LOGS_DIR environment variables, with file logging enabled automatically in non-TTY environments when a logs directory is specified.
Learnt from: haiqi96
PR: y-scope/clp#651
File: components/clp-package-utils/clp_package_utils/scripts/compress.py:0-0
Timestamp: 2025-01-16T16:58:43.190Z
Learning: In the clp-package compression flow, path validation and error handling is performed at the scheduler level rather than in the compress.py script to maintain simplicity and avoid code duplication.
🧬 Code Graph Analysis (1)
components/clp-package-utils/clp_package_utils/general.py (1)
components/clp-py-utils/clp_py_utils/clp_config.py (1)
CLPConfig(595-780)
🪛 Pylint (3.3.7)
components/clp-py-utils/clp_py_utils/clp_config.py
[convention] 587-587: Missing function or method docstring
(C0116)
[error] 587-587: Method 'validate_results_metadata_collection_name' should have "self" as first argument
(E0213)
components/clp-package-utils/clp_package_utils/scripts/start_clp.py
[convention] 875-875: Line too long (104/100)
(C0301)
[convention] 894-894: Line too long (104/100)
(C0301)
[convention] 844-844: Missing function or method docstring
(C0116)
[refactor] 844-844: Too many local variables (30/15)
(R0914)
[warning] 851-851: Use lazy % formatting in logging functions
(W1203)
[warning] 883-883: Using open without explicitly specifying an encoding
(W1514)
[refactor] 844-844: Too many statements (59/50)
(R0915)
[warning] 925-925: Using open without explicitly specifying an encoding
(W1514)
🔇 Additional comments (40)
components/webui/.gitignore (1)
3-3: Confirm the path accuracy foryscope-log-viewer/The ignore rule looks correct if a generated or legacy folder named
yscope-log-viewer/can appear directly insidecomponents/webui/.
If the directory no longer exists anywhere in the repo, you could drop this rule once the removal is fully complete. Otherwise, 👍 to keep build artefacts out of version control.components/webui/client/vite.config.ts (1)
27-27: LGTM! Comment accurately reflects the updated project structure.The comment correction aligns with the removal of the legacy log-viewer-webui component and consolidation under the webui directory.
components/webui/client/src/pages/IngestPage/Jobs/index.tsx (1)
9-9: LGTM! Import refactor improves code organisation.The import path change to use the centralised
api/sqlmodule enhances modularity and follows good separation of concerns.components/webui/client/src/pages/IngestPage/Details/index.tsx (1)
10-10: LGTM! Consistent import refactor.The import path change matches the refactoring pattern seen in other IngestPage components, maintaining consistency across the codebase.
components/webui/client/src/typings/query.ts (1)
52-52: All ExtractJobSearchParams usages include the new dataset parameter
Verified that inQueryStatus.tsx,parseResult.datasetis passed intosubmitExtractStreamJob. No further updates are required.components/webui/client/src/api/search.ts (1)
18-18: Verify inclusion of the new dataset field in all query submissionsAlthough QueryJobCreationSchema and submitQuery are correctly updated to require a dataset string, automated search across
.ts/.tsxdidn’t confirm that every call site builds a payload with this field. Please manually ensure that:
- In components/webui/client/src/pages/SearchPage/SearchControls/search-requests.ts, the object passed into handleQuerySubmit (in turn into submitQuery) includes the new dataset property.
- Any other invocations of submitQuery(payload) or constructions of QueryJobCreationSchema in
.ts/.tsxfiles explicitly set dataset.components/webui/client/src/pages/IngestPage/Jobs/sql.ts (2)
3-4: LGTM! Configuration centralization looks good.The import changes correctly align with the broader refactoring effort to centralize SQL configuration from individual modules into a shared
settingsmodule.
24-24: Consistent usage of centralized configuration.The SQL query correctly references the table name from the centralized
settingsmodule, maintaining consistency with the architectural changes.components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (1)
9-9: Good refactoring - API consolidation.Moving
querySqlto a centralized API module at../../../api/sqlimproves code organization and reusability across components.components/webui/client/src/pages/IngestPage/SpaceSavings/sql.ts (2)
1-2: Consistent configuration centralization.The import changes align with the broader effort to consolidate SQL configuration into a shared
settingsmodule, maintaining consistency across the IngestPage components.
24-24: Correct table name reference update.The SQL query properly references the archives table name from the centralized configuration, ensuring consistency with the architectural changes.
components/package-template/src/etc/clp-config.yml (1)
64-64: Good configuration cleanup and modernization.The removal of legacy
log_viewer_webuiconfiguration and addition ofresults_metadata_collection_nameproperly reflects the transition from the old log viewer to the new unified webui component.components/webui/client/src/main.tsx (1)
18-18: Excellent simplification of application entry point.Removing the conditional rendering logic and directly using the
Appcomponent eliminates unnecessary complexity and indicates successful completion of the UI migration.components/webui/client/package.json (1)
2-2: Package restructuring looks good.The renaming from "log-viewer-webui-client" to "webui-client" aligns with the consolidation of the webui component. The addition of React Query dependencies supports the new dataset functionality described in the PR.
Also applies to: 10-10, 21-22
components/clp-py-utils/clp_py_utils/s3_utils.py (1)
23-23: Component name update is correct.The replacement of
LOG_VIEWER_WEBUI_COMPONENT_NAMEwithWEBUI_COMPONENT_NAMEcorrectly aligns with the removal of the legacy log-viewer-webui component as described in the PR.Also applies to: 118-118
components/webui/client/src/pages/IngestPage/Details/sql.ts (1)
3-3: Configuration consolidation looks good.The migration from
SQL_CONFIGto centralizedsettingsfor table name references is consistent with the configuration restructuring mentioned in the AI summary.Also applies to: 26-26, 37-37
components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx (1)
11-11: Dataset integration implemented correctly.The addition of
cachedDatasetfrom the search store and its inclusion in the URL query parameters correctly supports the dataset selection feature. The use of a specific selector (state => state.cachedDataset) follows Zustand best practices for preventing unnecessary re-renders.Also applies to: 36-36, 50-51
components/webui/client/src/pages/SearchPage/SearchControls/SearchButton/index.module.css (1)
1-6: Clean gradient button styling.The CSS module provides a professional gradient design with smooth transitions. The
background-size: 200% autoproperty supports potential hover animations for enhanced user experience.components/webui/client/src/pages/SearchPage/SearchControls/index.tsx (2)
1-5: LGTM! Clean import organization and conditional rendering logic.The imports are well-organized and the conditional rendering of the Dataset component based on storage engine configuration follows a logical pattern that allows for engine-specific UI features.
30-30: Configuration constants are correctly defined and exportedI’ve verified that in
components/webui/client/src/config/index.ts:
enum CLP_STORAGE_ENGINESincludes bothCLPandCLP_Svariants.SETTINGS_STORAGE_ENGINEis defined assettings.ClpStorageEngine as CLP_STORAGE_ENGINES.- Both are exported alongside
STREAM_TYPE.No further changes are required here.
components/webui/client/src/api/sql.ts (1)
10-12: LGTM! Clean and type-safe SQL query utility.The generic function design allows for type-safe responses while keeping the implementation simple and reusable. The axios usage and endpoint structure are appropriate.
components/webui/client/src/settings.ts (1)
7-9: LGTM! Well-structured SQL table configuration properties.The new SQL database table name properties follow a consistent naming convention and appropriately extend the settings configuration for SQL database support.
components/webui/client/src/ui/QueryStatus.tsx (1)
68-79: LGTM! Excellent refactoring to use object parameters.The refactoring from multiple positional arguments to a single object parameter significantly improves readability and maintainability. The dataset integration is properly handled using parsed URL parameters.
components/webui/client/src/pages/SearchPage/SearchControls/SearchButton/index.tsx (2)
1-4: LGTM! Clean imports and component structure.The component imports are well-organized and the component follows a clear separation of concerns by delegating to specialized button components.
15-19: Consider improving readability of the conditional logic.While the ternary operator works correctly, the comparison could be more explicit and readable. Consider following the coding guidelines preference for explicit comparisons.
return ( - (searchUiState === SEARCH_UI_STATE.QUERYING) ? + (SEARCH_UI_STATE.QUERYING === searchUiState) ? <CancelButton/> : <SubmitButton/> );⛔ Skipped due to learnings
Learnt from: junhaoliao PR: y-scope/clp#937 File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24 Timestamp: 2025-05-29T20:33:40.653Z Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsx (4)
8-11: LGTM! Good addition of storage engine configuration imports.These imports support the new dataset-aware functionality for different storage engines.
26-36: Excellent refactor to use specific selectors!This change follows the established best practice of using specific selectors instead of destructuring the entire state object, which prevents unnecessary re-renders as the store grows in complexity.
65-73: Good validation logic for dataset requirement.The conditional check ensures that CLP_S queries include a required dataset, with appropriate error logging when missing. The early return prevents submission of invalid queries.
76-76: No signature change required for handleQuerySubmitThe
handleQuerySubmitfunction takes a singlepayload: QueryJobCreationSchema, and that type already includes adataset: stringfield (see components/webui/client/src/api/search.ts lines 17–22). Thedataset: selectDataset ?? ""you’re passing fits the existing signature—no update needed.Likely an incorrect or invalid review comment.
components/webui/client/src/App.tsx (2)
1-1: LGTM! Correct React Router v7 import.Using
react-routerpackage instead ofreact-router-domfollows the correct v7 structure.
23-34: Well-structured React app setup!The provider nesting order is correct: QueryClientProvider → ConfigProvider → RouterProvider. The ReactQueryDevtools placement outside the ConfigProvider is appropriate to avoid theming conflicts.
components/webui/client/public/settings.json (1)
1-7: Well-structured centralized configuration!This JSON file appropriately centralizes storage engine and database configuration that was previously scattered across the codebase. The naming conventions are clear and consistent.
components/webui/client/src/api/query.ts (3)
12-18: Good use of interface for function parameters.The
submitExtractStreamJobPropsinterface makes the function signature cleaner and more maintainable than individual parameters.
32-38: Well-structured parameter destructuring.The function signature using object destructuring with the interface improves readability and maintainability.
42-42: LGTM! Dataset parameter added to API payload.The dataset parameter is now included in the POST request, supporting the dataset-aware functionality.
components/clp-package-utils/clp_package_utils/general.py (1)
515-523: Good refactor to support separate client and server settings.The function signature change to accept
client_settings_json_pathandserver_settings_json_pathappropriately supports the new webui architecture. The validation logic correctly checks both settings files for existence.components/webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx (1)
41-52: Improve error handling for dataset validation.The current implementation logs an error but continues execution, which could lead to unexpected behavior. Additionally, the fallback to empty string on line 52 is redundant given the null check above.
Apply this diff to properly handle the error case:
- if (CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE) { - if (null !== selectDataset) { - updateCachedDataset(selectDataset); - } else { - console.error("Cannot submit a clp-s query without a dataset selection."); - - return; - } - } - - handleQuerySubmit({ - dataset: selectDataset ?? "", + if (CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE) { + if (null === selectDataset) { + console.error("Cannot submit a clp-s query without a dataset selection."); + return; + } + updateCachedDataset(selectDataset); + } + + handleQuerySubmit({ + dataset: selectDataset ?? "",⛔ Skipped due to learnings
Learnt from: junhaoliao PR: y-scope/clp#596 File: components/log-viewer-webui/client/src/api/query.js:16-23 Timestamp: 2024-11-21T15:51:33.203Z Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.components/webui/client/src/pages/SearchPage/SearchState/index.tsx (1)
19-19: Dataset state management implementation looks good!The addition of
cachedDatasetandselectDatasetstate properties with their corresponding update methods follows the existing patterns and properly separates the submitted dataset from the UI selection state.Also applies to: 26-26, 38-44, 75-79, 98-98, 105-105, 116-118, 137-139
components/clp-py-utils/clp_py_utils/clp_config.py (1)
586-592: Fix validator method signature.The validator method is missing
selfas the first parameter, which is required for instance methods.Apply this diff to fix the method signature:
- def validate_results_metadata_collection_name(cls, field): + def validate_results_metadata_collection_name(self, field):However, looking at other validators in this file (lines 99, 169, 196, etc.), they use
clsas they are class methods decorated with@validator. The correct fix is:@validator("results_metadata_collection_name") def validate_results_metadata_collection_name(cls, field):This follows the pydantic validator pattern used throughout the file.
Likely an incorrect or invalid review comment.
components/clp-package-utils/clp_package_utils/scripts/start_clp.py (1)
844-977: WebUI refactoring looks comprehensive!The removal of log-viewer-webui references and the updated configuration handling for separate client and server settings files is well-implemented. The addition of the
container_clp_configparameter properly separates host and container configurations.
components/webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/start_clp.py
Outdated
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/start_clp.py
Outdated
Show resolved
Hide resolved
Bill-hbrhbr
force-pushed
the
dataset-interface
branch
from
210c6f8 to
18ebff6
Compare
Bill-hbrhbr
changed the title
Description
This PR is deprecated due to merge conflicts that would bloat the PR size after resolution.
clp-package:
clp-package-utils
Add dataset arguments
--datasetto various package scripts‘ command-line interface. If no dataset argument is provided, the default dataset name isdefault.webui
Add dataset selector to new webui. Dataset selector has to query backend to get datasets, and then display them. I also now pass the dataset to the log viewer connector.
The
cachedDatasetis the dataset when query is submitted. We have to do this since the user could change the selector, then click a link, which would result in an error. Instead we cache the dataset when they submitted the query.Checklist
breaking change.
Validation performed
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Chores