feat: show workflow runs retention time in the UI#445
feat: show workflow runs retention time in the UI#445stefinie123 merged 2 commits intoopenchoreo:mainfrom
Conversation
- Introduced a new optional field `ttlAfterCompletion` in both `ClusterWorkflowEntityV1alpha1` and `WorkflowEntityV1alpha1` interfaces to specify the time-to-live for completed WorkflowRun instances. - Updated `OpenChoreoEntityProvider` to handle the new `ttlAfterCompletion` field during entity translation. - Added a custom hook `useWorkflowRetention` to fetch the `ttlAfterCompletion` value from the catalog entity and format it for display in the UI. - Enhanced `RunsTab` and `Workflows` components to utilize the retention TTL, providing users with information on run retention periods. Signed-off-by: Stefinie Fernando <minolispencer@gmail.com>
📝 WalkthroughWalkthroughAdds optional Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Workflows Component
participant Hook as useWorkflowRetention Hook
participant API as Catalog API
participant Backend as Catalog Backend
participant RunsTab as RunsTab Component
UI->>Hook: call useWorkflowRetention(workflowName, kind, namespace)
activate Hook
Hook->>API: catalogApi.getEntityByRef(workflowRef)
activate API
API->>Backend: fetch workflow entity
activate Backend
Backend-->>API: return WorkflowEntityV1alpha1 with spec.ttlAfterCompletion
deactivate Backend
API-->>Hook: entity data
deactivate API
Hook->>Hook: extract entity.spec.ttlAfterCompletion into ttl state
deactivate Hook
Hook-->>UI: return retentionTtl
UI->>RunsTab: pass retentionTtl prop
activate RunsTab
RunsTab->>RunsTab: formatRetentionDuration(retentionTtl)
RunsTab->>RunsTab: render tooltip & empty-state message
deactivate RunsTab
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/openchoreo-ci/src/components/RunsTab/RunsTab.tsx`:
- Around line 86-103: The InfoOutlinedIcon used as the Tooltip trigger is not
keyboard-focusable; wrap the icon in a focusable control like Material-UI's
IconButton and supply an aria-label so keyboard and screen-reader users can open
the tooltip. Locate the block that checks retentionTtl and uses
formatRetentionDuration and replace the raw InfoOutlinedIcon with an IconButton
(containing InfoOutlinedIcon) that has a meaningful aria-label (e.g., "Retention
period info") and preserves the existing styling (fontSize/color/margin) so the
Tooltip still attaches to the interactive element.
In `@plugins/openchoreo-ci/src/hooks/useWorkflowRetention.ts`:
- Around line 24-44: The fetchTtl function can leave stale TTL state when the
lookup fails or returns no entity; modify fetchTtl to clear the stored TTL by
calling setTtl(undefined) whenever the entityNamespace is missing, when
catalogApi.getEntityByRef throws, or when no entity/spec/ttlAfterCompletion is
found (i.e., any early return or catch path). Update the logic around
entityNamespace, the post-getEntityByRef block, and the catch handler in
fetchTtl (and reference ignore, catalogApi.getEntityByRef, workflowKind,
namespace, workflowName, setTtl) so that any unresolved lookup explicitly resets
TTL instead of leaving previous values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a809cef2-d8b1-40c0-94de-6f4562dca32c
📒 Files selected for processing (8)
plugins/catalog-backend-module-openchoreo/src/kinds/ClusterWorkflowEntityV1alpha1.tsplugins/catalog-backend-module-openchoreo/src/kinds/WorkflowEntityV1alpha1.tsplugins/catalog-backend-module-openchoreo/src/provider/OpenChoreoEntityProvider.tsplugins/catalog-backend-module-openchoreo/src/utils/entityTranslation.tsplugins/openchoreo-ci/src/components/RunsTab/RunsTab.tsxplugins/openchoreo-ci/src/components/Workflows/Workflows.tsxplugins/openchoreo-ci/src/hooks/index.tsplugins/openchoreo-ci/src/hooks/useWorkflowRetention.ts
- Replaced InfoOutlinedIcon with an IconButton in the RunsTab component for better accessibility and styling. - Updated useWorkflowRetention hook to ensure TTL is set correctly based on entity existence and ignore flag. Signed-off-by: Stefinie Fernando <minolispencer@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/openchoreo-ci/src/hooks/useWorkflowRetention.ts`:
- Around line 71-77: The current duration formatting pushes days, hours and
sometimes minutes (variables days, hours, minutes) resulting in "10 days 1 hour"
for "10d1h30m"; change the logic in useWorkflowRetention (the formatter in
plugins/openchoreo-ci/src/hooks/useWorkflowRetention.ts) to only show the
largest non-zero unit: if days > 0 push only the days string (with proper
pluralization) and return; else if hours > 0 push only the hours string; else
push minutes (or fall back to duration) so the output for "10d1h30m" becomes "10
days" and matches the documented concise example.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0cc3a522-0071-4099-bb08-1dd7f612c8b4
📒 Files selected for processing (2)
plugins/openchoreo-ci/src/components/RunsTab/RunsTab.tsxplugins/openchoreo-ci/src/hooks/useWorkflowRetention.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/openchoreo-ci/src/components/RunsTab/RunsTab.tsx
| if (days > 0) parts.push(`${days} ${days === 1 ? 'day' : 'days'}`); | ||
| if (hours > 0) parts.push(`${hours} ${hours === 1 ? 'hour' : 'hours'}`); | ||
| if (minutes > 0 && days === 0) | ||
| parts.push(`${minutes} ${minutes === 1 ? 'minute' : 'minutes'}`); | ||
|
|
||
| return parts.length > 0 ? parts.join(' ') : duration; | ||
| } |
There was a problem hiding this comment.
Formatter output is inconsistent with its documented example.
For input 10d1h30m, current logic returns 10 days 1 hour, while the function docs describe a concise output like 10 days. Aligning behavior with the docs will keep tooltip/empty-state copy consistent.
💡 Proposed fix
- const parts: string[] = [];
- if (days > 0) parts.push(`${days} ${days === 1 ? 'day' : 'days'}`);
- if (hours > 0) parts.push(`${hours} ${hours === 1 ? 'hour' : 'hours'}`);
- if (minutes > 0 && days === 0)
- parts.push(`${minutes} ${minutes === 1 ? 'minute' : 'minutes'}`);
-
- return parts.length > 0 ? parts.join(' ') : duration;
+ if (days > 0) return `${days} ${days === 1 ? 'day' : 'days'}`;
+ if (hours > 0) return `${hours} ${hours === 1 ? 'hour' : 'hours'}`;
+ if (minutes > 0) return `${minutes} ${minutes === 1 ? 'minute' : 'minutes'}`;
+ return duration;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/openchoreo-ci/src/hooks/useWorkflowRetention.ts` around lines 71 -
77, The current duration formatting pushes days, hours and sometimes minutes
(variables days, hours, minutes) resulting in "10 days 1 hour" for "10d1h30m";
change the logic in useWorkflowRetention (the formatter in
plugins/openchoreo-ci/src/hooks/useWorkflowRetention.ts) to only show the
largest non-zero unit: if days > 0 push only the days string (with proper
pluralization) and return; else if hours > 0 push only the hours string; else
push minutes (or fall back to duration) so the output for "10d1h30m" becomes "10
days" and matches the documented concise example.
Purpose
Show workflow runs retention time in the UI
Approach
This pull request adds support for displaying and propagating the workflow run retention time-to-live (TTL) in the OpenChoreo workflow system. It introduces a new field,
ttlAfterCompletion, to both workflow entity types and ensures this retention information is fetched from the catalog and shown in the UI, improving transparency for users about how long workflow run data is retained.API and Data Model Enhancements:
ttlAfterCompletionfield (a duration string, e.g., "10d1h30m") to bothWorkflowEntityV1alpha1andClusterWorkflowEntityV1alpha1interfaces to represent workflow run retention after completion. [1] [2]ttlAfterCompletionfrom backend workflow specs into catalog entities, ensuring the value is available throughout the stack. [1] [2] [3] [4] [5] [6] [7] [8]Frontend and UI Improvements:
useWorkflowRetention, to fetch thettlAfterCompletionvalue from the catalog entity, and a utility,formatRetentionDuration, for displaying the duration in a human-readable format. [1] [2]RunsTabcomponent to display workflow run retention information via an info tooltip and a message in the empty state, using the fetched TTL value. [1] [2] [3] [4] [5]Workflowscomponent, wiring the new hook and passing the value toRunsTabfor display. [1] [2] [3]These changes ensure that workflow retention policies are clearly communicated to users and consistently handled in both backend processing and frontend display.
Summary by CodeRabbit