goutamadwant commented on code in PR #11818:
URL: https://github.com/apache/seatunnel/pull/11818#discussion_r3790074248
##########
seatunnel-engine/seatunnel-engine-ui/src/service/job/index.ts:
##########
@@ -20,7 +20,7 @@ import type { Job, JobPage } from './types'
export const getRunningJobs = (page: number, rows: number) =>
get<JobPage>('/running-jobs/summary', {page: page, rows: rows})
-export const getFinishedJobs = (page: number, rows: number) =>
get<JobPage>(`/finished-jobs`, {page: page, rows: rows})
+export const getFinishedJobs = (page: number, rows: number) =>
get<JobPage>(`/finished-jobs/finished`, {page: page, rows: rows})
Review Comment:
Changing this URL adds a FINISHED state filter; it does not repair the
current /finished-jobs endpoint. JobInfoService#getJobsByStateJson returns
every terminal state when the path state is empty, while
/finished-jobs/finished keeps only FINISHED. This hides FAILED, CANCELED,
SAVEPOINT_DONE, and UNKNOWABLE jobs from the Finished Jobs page and removes
access to their exception and detail views.
I reproduced this on the current PR head.
RestApiHttpsTest#testFinishedJobsApi passes and asserts that
/finished-jobs?page=1&rows=5 returns 200. A focused service test with one
FAILED and one FINISHED record returned both jobs without a state filter and
only the successful job with finished. The new unit test only checks the
requested URL, so it cannot detect this data loss.
Please keep the UI on /finished-jobs and reproduce the reported 500 against
current dev. If it is data-dependent, fix that server-side and add a regression
for the terminal record that triggers it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
