feat: add build output history log echo functionality#1580
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the build output history log functionality to use a centralized API client (orionApiClient) instead of direct fetch calls, improving code consistency and maintainability. The changes also include replacing hardcoded URL logic with dynamic base URL resolution and cleaning up commented-out code.
Key Changes
- Migrated from direct
fetchcalls toorionApiClientfor task and log retrieval - Replaced hardcoded URL conditionals with dynamic base URL resolution from
orionApiClient - Cleaned up unused types, commented code, and improved error messages
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| moon/apps/web/hooks/SSE/useGetHTTPLog.ts | Refactored to use orionApiClient instead of direct HTTP calls; added fetchHTTPLog export |
| moon/apps/web/hooks/SSE/useGetClTask.ts | Migrated to orionApiClient and removed duplicate type definitions |
| moon/apps/web/hooks/SSE/ssmRequest.ts | Removed fetchTask and HttpTaskRes functions; replaced hardcoded paths with dynamic base URL |
| moon/apps/web/components/ClView/hook/useSSM.ts | Replaced hardcoded URL conditionals with getTaskOutputSSEUrl helper; improved error messages |
| moon/apps/web/components/ClView/components/Checks/index.tsx | Simplified log fetching logic; improved UI feedback for loading and empty states |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import { SSEPATH } from '@/components/ClView/hook/useSSM' | ||
| import { orionApiClient } from '@/utils/queryClient' | ||
|
|
||
| const getBaseUrl = () => (orionApiClient as any).baseUrl || '' |
There was a problem hiding this comment.
The use of (orionApiClient as any) bypasses type safety. Consider adding a proper type definition or accessor method to orionApiClient to expose baseUrl in a type-safe manner.
| * Path: GET:/task-output/{id} | ||
| */ | ||
| export const getTaskOutputSSEUrl = (taskId: string) => { | ||
| const baseUrl = (orionApiClient as any).baseUrl || '' |
There was a problem hiding this comment.
The use of (orionApiClient as any) bypasses type safety. Consider adding a proper type definition or accessor method to orionApiClient to expose baseUrl in a type-safe manner.
| window.location.href.includes('app') | ||
| ? (sseUrl.current = 'https://orion.gitmega.com/logs?follow=true') | ||
| : (sseUrl.current = '/sse/logs?follow=true') | ||
| const baseUrl = (orionApiClient as any).baseUrl || '' |
There was a problem hiding this comment.
The use of (orionApiClient as any) bypasses type safety. Consider adding a proper type definition or accessor method to orionApiClient to expose baseUrl in a type-safe manner.
| acc[i.value.task_id] = i.value.data === '' ? 'empty logs, please check it later' : i.value.data | ||
| (acc, item) => { | ||
| if (item.status === 'fulfilled' && item.value) { | ||
| const { id, res } = item.value |
There was a problem hiding this comment.
The conditional logic assumes res.data could be either an array or a string, but this dual behavior is unclear. Consider adding a comment explaining when res.data is an array vs. a string, or standardizing the API response format.
| const { id, res } = item.value | |
| const { id, res } = item.value | |
| // NOTE: res.data can be either an array (of log lines) or a string, depending on the API response. | |
| // If the API returns an array, we join the lines; if it's a string, we use it directly. | |
| // This dual behavior is due to inconsistent API responses. Update this logic if the API is standardized. |
build output history log echo functionality