feat: Concurrent executions with indexedDB #1235
Conversation
|
I know the test are not working currently. Its just to show the feature, i will work on the test. I am having trouble with getting test of redux to work 😕 |
|
@Microchesst thanks for the PR. I will get back to you by Monday. |
There was a problem hiding this comment.
@Microchesst Thanks for the PR. The feature looks really nice. Please see the comments.
There was a problem hiding this comment.
please move all the *.ts files in route/digitaltwins/execute to model/backend/gitlab/execution directory. Do remember to
- create an interface for each class so that testing becomes easier. The interfaces related to pipelines can be kept in
model/backend/gitlab/execution/interfaces.ts - always have
preview/code depend on stable codebase rather than other way around
There was a problem hiding this comment.
This file can be placed in src/route/digitaltwins.ts. Do remember to create an inerface / type for the IndexDBService class.
There was a problem hiding this comment.
make src/database/digitaltwins.ts
There was a problem hiding this comment.
Since this is a redux slice, can it be placed in model/...?
There was a problem hiding this comment.
Do think about the appropriate location for these slices. Is it src/store or src/model/store?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/distributed-demo #1235 +/- ##
============================================================
- Coverage 92.08% 89.97% -2.11%
============================================================
Files 92 96 +4
Lines 2526 2982 +456
Branches 406 497 +91
============================================================
+ Hits 2326 2683 +357
- Misses 197 296 +99
Partials 3 3
... and 1 file with indirect coverage changes
🚀 New features to boost your workflow:
|
|
@Microchesst please move your pure typescript files (*.ts) to |
|
The redux slices can be placed on |
There was a problem hiding this comment.
please change the following line.
"test:e2e": "yarn config:test && yarn build:prod && playwright test -c ./playwright.config.ts",
There was a problem hiding this comment.
Pull Request Overview
This PR adds concurrent execution support for Digital Twins by persisting and managing execution history in IndexedDB and enhancing the UI to display and control multiple runs.
- Migrate digital-twin state and actions to
model/backend/gitlabslices and introduce IndexedDB schema/service. - Introduce
ExecutionHistoryLoaderfor initial load and polling, plusExecutionHistoryList, updated dialogs/buttons to surface history. - Extend pipeline handlers (
pipelineUtils,pipelineHandler,pipelineChecks) to create, track, and update concurrent executions.
Reviewed Changes
Copilot reviewed 65 out of 65 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/preview/route/digitaltwins/execute/LogDialog.tsx | Fetch execution history via thunk and render ExecutionHistoryList instead of inline logs |
| client/src/preview/route/digitaltwins/editor/Sidebar.tsx | Updated selectDigitalTwinByName import to new model/backend slice |
| client/src/preview/route/digitaltwins/create/CreateDTDialog.tsx | Switched setDigitalTwin import to new model/backend slice |
| client/src/preview/components/execution/ExecutionHistoryLoader.tsx | New component to load all history on startup and poll running executions |
| client/src/preview/components/execution/ExecutionHistoryList.tsx | New list component displaying sorted history with stop/delete controls and log details |
| client/src/preview/components/asset/StartStopButton.tsx | Show running count, integrate handleStart, and replace loading indicator |
| client/src/preview/components/asset/LogButton.tsx | Add badge count for history entries and disable logic based on execution count |
| client/src/preview/components/asset/DetailsButton.tsx | Updated selectDigitalTwinByName import path |
| client/src/preview/components/asset/AssetCard.tsx | Initialize logButtonDisabled to false and pass assetName to LogButton |
| client/src/preview/components/asset/AssetBoard.tsx | Updated setShouldFetchDigitalTwins import to new model/backend slice |
| client/src/model/backend/gitlab/types/executionHistory.ts | New ExecutionStatus, JobLog, ExecutionHistoryEntry, and IndexedDB schema/types |
| client/src/model/backend/gitlab/state/executionHistory.slice.ts | New slice with CRUD thunks, selectors, and polling logic for concurrent execution tracking |
| client/src/model/backend/gitlab/state/digitalTwin.slice.ts | Aligned JobLog type import and added selectDigitalTwins selector |
| client/src/model/backend/gitlab/execution/pipelineUtils.ts | Refactored pipeline state helpers to support execution history updates and concurrent flows |
| client/src/model/backend/gitlab/execution/pipelineHandler.ts | Updated handleStart/handleStop to dispatch history fetch and pass executionId |
| client/src/model/backend/gitlab/execution/pipelineChecks.ts | Enhanced status checks to update IndexedDB, handle timeouts, and use new interfaces |
| client/src/model/backend/gitlab/execution/interfaces.ts | Defined PipelineStatusParams and PipelineHandlerDispatch for typed thunk usage |
| client/src/database/digitalTwins.ts | IndexedDB service implementation conforming to new schema for persisting execution history |
| client/src/AppProvider.tsx | Registered ExecutionHistoryLoader at the app root |
| client/package.json | Added uuid, @types/uuid, fake-indexeddb and updated dependencies for IndexedDB and IDs |
Comments suppressed due to low confidence (3)
client/src/preview/components/execution/ExecutionHistoryLoader.tsx:15
ExecutionHistoryLoaderhandles initial data fetch and polling but has no associated unit or integration tests. Consider adding tests to cover its effect logic and cleanup of the interval.
const ExecutionHistoryLoader: React.FC = () => {
client/src/preview/route/digitaltwins/execute/LogDialog.tsx:27
- [nitpick] The component is named
LogDialogbut now presents execution history. Consider renaming it toExecutionHistoryDialogfor clarity and consistency with its content.
function LogDialog({ showLog, setShowLog, name }: LogDialogProps) {
client/src/model/backend/gitlab/execution/pipelineChecks.ts:76
- Calling
digitalTwin.getExecutionHistoryByIdassumes theDigitalTwinclass exposes that method, but IndexedDB access should useindexedDBService. Replace this withindexedDBService.getExecutionHistoryByIdto avoid runtime errors.
const pipelineId = executionId
| const setButtonText = () => {}; // Dummy function since we don't need to change button text | ||
| handleStart( | ||
| 'Start', |
There was a problem hiding this comment.
The call to handleStart always passes 'Start' and you’ve removed any buttonText state, so the UI can never switch to 'Stop'. This breaks the stop functionality—consider managing a local buttonText state or passing the current executionId to toggle correctly.
| const setButtonText = () => {}; // Dummy function since we don't need to change button text | |
| handleStart( | |
| 'Start', | |
| const newButtonText = buttonText === 'Start' ? 'Stop' : 'Start'; | |
| setButtonText(newButtonText); | |
| handleStart( | |
| newButtonText, |
Copilot uses AI. Check for mistakes.
| } | ||
| }, [dispatch, name, showLog]); | ||
|
|
||
| const handleViewLogs = () => {}; |
There was a problem hiding this comment.
handleViewLogs is defined with no parameters but ExecutionHistoryList expects onViewLogs to accept an executionId: string. Either implement log fetching there or remove the unused callback.
| const handleViewLogs = () => {}; | |
| const handleViewLogs = (executionId: string) => { | |
| console.log(`View logs for execution ID: ${executionId}`); | |
| }; |
Copilot uses AI. Check for mistakes.
| (jobLog: JobLog, index: number) => ( | ||
| <div key={index} style={{ marginBottom: '16px' }}> |
There was a problem hiding this comment.
[nitpick] Using the array index as a React key can lead to rendering issues if logs change order or new logs are inserted. Use a stable identifier (e.g., jobLog.jobName or a combined ID) instead.
| (jobLog: JobLog, index: number) => ( | |
| <div key={index} style={{ marginBottom: '16px' }}> | |
| (jobLog: JobLog) => ( | |
| <div key={jobLog.jobName} style={{ marginBottom: '16px' }}> |
Copilot uses AI. Check for mistakes.
Pull Request Template
Title
Implement Concurrent Digital Twin Execution with IndexedDB Integration
Type of Change
Description
This PR implements concurrent execution for Digital Twins with IndexedDB integration. Users can now start multiple executions of the same Digital Twin simultaneously, with execution history persisted in the browser's IndexedDB. The implementation includes a new execution history data model, IndexedDB service, and UI components to display and manage execution history.
##Working on Testing
Impact
Checklist