Skip to content

Commit

Permalink
handle uncaught error in batch actions (#1184)
Browse files Browse the repository at this point in the history
* handle uncaught error in batch actions

* clean up batch/bulk code

* fix mocks for tests
  • Loading branch information
rossedfort committed Feb 24, 2023
1 parent 7eead81 commit 0115d67
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import { pluralize } from '$lib/utilities/pluralize';
import Input from '$lib/holocene/input/input.svelte';
export let action: 'Terminate' | 'Cancel';
export let loading: boolean;
export let allSelected: boolean;
export let actionableWorkflowsLength: number;
export let query: string;
Expand Down Expand Up @@ -36,7 +35,6 @@
confirmType="destructive"
confirmDisabled={reason === ''}
{confirmText}
{loading}
on:cancelModal={handleCancelModal}
on:confirmModal={handleConfirmModal}
>
Expand Down
84 changes: 20 additions & 64 deletions src/lib/pages/workflows-with-new-search.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
let batchCancelConfirmationModal: BatchOperationConfirmationModal;
let allSelected: boolean = false;
let pageSelected: boolean = false;
let terminating: boolean = false;
$: query = $page.url.searchParams.get('query');
Expand Down Expand Up @@ -91,8 +90,6 @@
};
const resetPageToDefaultState = () => {
terminating = false;
$workflowFilters = [];
$workflowSorts = [];
updateQueryParameters({
Expand Down Expand Up @@ -133,82 +130,43 @@
};
const terminateWorkflows = async (event: CustomEvent<{ reason: string }>) => {
const { namespace } = $page.params;
const { reason } = event.detail;
const options = {
namespace: $page.params.namespace,
reason: event.detail.reason,
};
if (allSelected) {
// Idea: persist the job ID and display a progress indicator for large jobs
await batchTerminateByQuery({
namespace,
reason,
...options,
query: batchOperationQuery,
});
toaster.push({
message: 'The batch terminate request is processing in the background.',
id: 'batch-terminate-success-toast',
});
} else {
terminating = true;
try {
const jobId = await bulkTerminateByIDs({
namespace,
reason,
workflows: terminableWorkflows,
});
const workflowsTerminated = await pollBatchOperation({
namespace,
jobId,
});
toaster.push({
message: `Successfully terminated ${workflowsTerminated} workflows.`,
id: 'batch-terminate-success-toast',
});
} catch (error) {
toaster.push({
variant: 'error',
message: 'Unable to terminate workflows.',
});
}
await bulkTerminateByIDs({
...options,
workflows: terminableWorkflows,
});
}
resetPageToDefaultState();
};
const cancelWorkflows = async (event: CustomEvent<{ reason: string }>) => {
const { namespace } = $page.params;
const { reason } = event.detail;
const options = {
namespace: $page.params.namespace,
reason: event.detail.reason,
};
if (allSelected) {
await batchCancelByQuery({
namespace,
reason,
...options,
query: batchOperationQuery,
});
toaster.push({
message: 'The batch cancel request is processing in the background.',
id: 'batch-cancel-success-toast',
});
} else {
try {
const jobId = await bulkCancelByIDs({
namespace,
reason,
workflows: cancelableWorkflows,
});
const workflowsCancelled = await pollBatchOperation({
namespace,
jobId,
});
toaster.push({
message: `Successfully cancelled ${workflowsCancelled} workflows.`,
id: 'batch-cancel-success-toast',
});
} catch {
toaster.push({
variant: 'error',
message: 'Unable to cancel workflows.',
});
}
await bulkCancelByIDs({
...options,
workflows: cancelableWorkflows,
});
}
resetPageToDefaultState();
};
Expand Down Expand Up @@ -246,7 +204,6 @@
<BatchOperationConfirmationModal
action="Terminate"
bind:this={batchTerminateConfirmationModal}
loading={terminating}
{allSelected}
actionableWorkflowsLength={terminableWorkflows.length}
query={batchOperationQuery}
Expand All @@ -255,7 +212,6 @@
<BatchOperationConfirmationModal
action="Cancel"
bind:this={batchCancelConfirmationModal}
loading={false}
{allSelected}
actionableWorkflowsLength={cancelableWorkflows.length}
query={batchOperationQuery}
Expand Down
20 changes: 14 additions & 6 deletions src/lib/services/batch-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,15 @@ const mockWorkflows = [
];

vi.mock('../utilities/request-from-api', () => ({
requestFromAPI: vi.fn().mockResolvedValue({ jobId: 'abc-123' }),
requestFromAPI: vi.fn().mockImplementation((route) => {
if (route.includes('/describe')) {
return new Promise((resolve) =>
resolve({ state: 'Completed', completedOperationCount: '10' }),
);
}

return new Promise((resolve) => resolve('xxx'));
}),
}));

vi.mock('uuid', () => ({
Expand All @@ -27,7 +35,7 @@ vi.mock('../stores/versions', () => {

describe('Batch Service', () => {
afterEach(() => {
vi.restoreAllMocks();
vi.clearAllMocks();
});

describe('when temporal server version is <= 1.19', () => {
Expand All @@ -44,7 +52,7 @@ describe('Batch Service', () => {
workflows: mockWorkflows,
});

expect(requestFromAPI).toHaveBeenCalledOnce();
expect(requestFromAPI).toHaveBeenCalledTimes(2);
expect(requestFromAPI).toHaveBeenCalledWith(
'http://localhost:8233/api/v1/namespaces/default/batch-operations',
{
Expand All @@ -65,7 +73,7 @@ describe('Batch Service', () => {
workflows: mockWorkflows,
});

expect(requestFromAPI).toHaveBeenCalledOnce();
expect(requestFromAPI).toHaveBeenCalledTimes(2);
expect(requestFromAPI).toHaveBeenCalledWith(
'http://localhost:8233/api/v1/namespaces/default/batch-operations',
{
Expand Down Expand Up @@ -94,7 +102,7 @@ describe('Batch Service', () => {
workflows: mockWorkflows,
});

expect(requestFromAPI).toHaveBeenCalledOnce();
expect(requestFromAPI).toHaveBeenCalledTimes(2);
expect(requestFromAPI).toHaveBeenCalledWith(
'http://localhost:8233/api/v1/namespaces/default/batch-operations',
{
Expand All @@ -115,7 +123,7 @@ describe('Batch Service', () => {
workflows: mockWorkflows,
});

expect(requestFromAPI).toHaveBeenCalledOnce();
expect(requestFromAPI).toHaveBeenCalledTimes(2);
expect(requestFromAPI).toHaveBeenCalledWith(
'http://localhost:8233/api/v1/namespaces/default/batch-operations',
{
Expand Down
73 changes: 63 additions & 10 deletions src/lib/services/batch-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {
} from 'src/types';
import { isVersionNewer } from '$lib/utilities/version-check';
import { temporalVersion } from '$lib/stores/versions';
import { toaster } from '$lib/stores/toaster';

type CreateBatchOperationOptions = {
namespace: string;
Expand Down Expand Up @@ -74,32 +75,84 @@ const createBatchOperationOptions = ({

export async function bulkTerminateByIDs(
options: CreateBatchOperationWithIDsOptions,
) {
const fullOptions = createBatchOperationOptions(options);
return terminateWorkflows(fullOptions);
): Promise<void> {
try {
const fullOptions = createBatchOperationOptions(options);
const jobId = await terminateWorkflows(fullOptions);
const workflowsTerminated = await pollBatchOperation({
namespace: fullOptions.namespace,
jobId,
});
toaster.push({
message: `Successfully terminated ${workflowsTerminated} workflows.`,
id: 'batch-terminate-success-toast',
});
} catch {
toaster.push({
message: 'Unable to terminate Workflows.',
variant: 'error',
});
}
}

export async function batchTerminateByQuery({
namespace,
query,
reason,
}: CreateBatchOperationWithQueryOptions) {
return terminateWorkflows({ namespace, query, reason });
}: CreateBatchOperationWithQueryOptions): Promise<void> {
try {
await terminateWorkflows({ namespace, query, reason });
toaster.push({
message: 'The batch terminate request is processing in the background.',
id: 'batch-terminate-success-toast',
});
} catch {
toaster.push({
message: 'Unable to terminate Workflows.',
variant: 'error',
});
}
}

export async function bulkCancelByIDs(
options: CreateBatchOperationWithIDsOptions,
): Promise<string> {
const fullOptions = createBatchOperationOptions(options);
return cancelWorkflows(fullOptions);
): Promise<void> {
try {
const fullOptions = createBatchOperationOptions(options);
const jobId = await cancelWorkflows(fullOptions);
const workflowsCancelled = await pollBatchOperation({
namespace: fullOptions.namespace,
jobId,
});
toaster.push({
message: `Successfully cancelled ${workflowsCancelled} workflows.`,
id: 'batch-cancel-success-toast',
});
} catch {
toaster.push({
message: 'Unable to cancel Workflows.',
variant: 'error',
});
}
}

export async function batchCancelByQuery({
namespace,
query,
reason,
}: CreateBatchOperationWithQueryOptions) {
return cancelWorkflows({ namespace, query, reason });
}: CreateBatchOperationWithQueryOptions): Promise<void> {
try {
await cancelWorkflows({ namespace, query, reason });
toaster.push({
message: 'The batch cancel request is processing in the background.',
id: 'batch-cancel-success-toast',
});
} catch {
toaster.push({
message: 'Unable to cancel Workflows.',
variant: 'error',
});
}
}

async function cancelWorkflows({
Expand Down

0 comments on commit 0115d67

Please sign in to comment.