Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit a4d3da4

Browse files
authored
Allow to cancel running external service sync jobs (#41518)
This PR makes use of dbworker cancellation which we recently added as a feature to allow to cancel a running external service sync, so customers don't need to shoot down their instance in case of misconfiguration (eg when syncing the entirety of github.com). It also contains a couple of tweaks to honor context cancellation better in the sync jobs, so that we bail out early. Worker cancellation is async, and the UI handles that with a canceling state.
1 parent 97051e7 commit a4d3da4

24 files changed

+665
-74
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
.cancel-button {
2+
margin-right: 0.5rem;
3+
justify-content: flex-end;
4+
align-self: flex-start;
5+
}

client/web/src/components/externalServices/ExternalServicePage.story.tsx

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { DecoratorFn, Story, Meta } from '@storybook/react'
2+
import { subMinutes } from 'date-fns'
23
import { of } from 'rxjs'
34
import { MATCH_ANY_PARAMETERS, WildcardMockLink } from 'wildcard-mock-link'
45

56
import { getDocumentNode } from '@sourcegraph/http-client'
67
import { NOOP_TELEMETRY_SERVICE } from '@sourcegraph/shared/src/telemetry/telemetryService'
78
import { MockedTestProvider } from '@sourcegraph/shared/src/testing/apollo'
89

9-
import { ExternalServiceFields, ExternalServiceKind } from '../../graphql-operations'
10+
import { ExternalServiceFields, ExternalServiceKind, ExternalServiceSyncJobState } from '../../graphql-operations'
1011
import { WebStory } from '../WebStory'
1112

1213
import { FETCH_EXTERNAL_SERVICE, queryExternalServiceSyncJobs as _queryExternalServiceSyncJobs } from './backend'
@@ -51,9 +52,42 @@ const externalService = {
5152

5253
const queryExternalServiceSyncJobs: typeof _queryExternalServiceSyncJobs = () =>
5354
of({
54-
totalCount: 0,
55+
totalCount: 4,
5556
pageInfo: { endCursor: null, hasNextPage: false },
56-
nodes: [],
57+
nodes: [
58+
{
59+
__typename: 'ExternalServiceSyncJob',
60+
failureMessage: null,
61+
startedAt: subMinutes(new Date(), 25).toISOString(),
62+
finishedAt: null,
63+
id: 'SYNCJOB1',
64+
state: ExternalServiceSyncJobState.CANCELING,
65+
},
66+
{
67+
__typename: 'ExternalServiceSyncJob',
68+
failureMessage: null,
69+
startedAt: subMinutes(new Date(), 25).toISOString(),
70+
finishedAt: null,
71+
id: 'SYNCJOB1',
72+
state: ExternalServiceSyncJobState.PROCESSING,
73+
},
74+
{
75+
__typename: 'ExternalServiceSyncJob',
76+
failureMessage: 'Very bad error syncing with the code host.',
77+
startedAt: subMinutes(new Date(), 25).toISOString(),
78+
finishedAt: subMinutes(new Date(), 25).toISOString(),
79+
id: 'SYNCJOB1',
80+
state: ExternalServiceSyncJobState.FAILED,
81+
},
82+
{
83+
__typename: 'ExternalServiceSyncJob',
84+
failureMessage: null,
85+
startedAt: subMinutes(new Date(), 25).toISOString(),
86+
finishedAt: subMinutes(new Date(), 25).toISOString(),
87+
id: 'SYNCJOB1',
88+
state: ExternalServiceSyncJobState.COMPLETED,
89+
},
90+
],
5791
})
5892

5993
function newFetchMock(node: { __typename: 'ExternalService' } & ExternalServiceFields): WildcardMockLink {

client/web/src/components/externalServices/ExternalServicePage.tsx

Lines changed: 81 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import React, { useEffect, useState, useCallback, useMemo } from 'react'
33
import * as H from 'history'
44
import { parse as parseJSONC } from 'jsonc-parser'
55
import { Redirect, useHistory } from 'react-router'
6-
import { Observable, Subject } from 'rxjs'
6+
import { Subject } from 'rxjs'
7+
import { delay, repeatWhen } from 'rxjs/operators'
78

89
import { ErrorAlert } from '@sourcegraph/branded/src/components/alerts'
910
import { hasProperty } from '@sourcegraph/common'
@@ -20,6 +21,7 @@ import {
2021
ExternalServiceSyncJobConnectionFields,
2122
ExternalServiceResult,
2223
ExternalServiceVariables,
24+
ExternalServiceSyncJobState,
2325
} from '../../graphql-operations'
2426
import { FilteredConnection, FilteredConnectionQueryArguments } from '../FilteredConnection'
2527
import { LoaderButton } from '../LoaderButton'
@@ -32,12 +34,15 @@ import {
3234
queryExternalServiceSyncJobs as _queryExternalServiceSyncJobs,
3335
useUpdateExternalService,
3436
FETCH_EXTERNAL_SERVICE,
37+
useCancelExternalServiceSync,
3538
} from './backend'
3639
import { ExternalServiceCard } from './ExternalServiceCard'
3740
import { ExternalServiceForm } from './ExternalServiceForm'
3841
import { defaultExternalServices, codeHostExternalServices } from './externalServices'
3942
import { ExternalServiceWebhook } from './ExternalServiceWebhook'
4043

44+
import styles from './ExternalServicePage.module.scss'
45+
4146
interface Props extends TelemetryProps {
4247
externalServiceID: Scalars['ID']
4348
isLightTheme: boolean
@@ -235,7 +240,7 @@ export const ExternalServicePage: React.FunctionComponent<React.PropsWithChildre
235240

236241
interface ExternalServiceSyncJobsListProps {
237242
externalServiceID: Scalars['ID']
238-
updates: Observable<void>
243+
updates: Subject<void>
239244

240245
/** For testing only. */
241246
queryExternalServiceSyncJobs?: typeof _queryExternalServiceSyncJobs
@@ -251,7 +256,7 @@ const ExternalServiceSyncJobsList: React.FunctionComponent<ExternalServiceSyncJo
251256
queryExternalServiceSyncJobs({
252257
first: args.first ?? null,
253258
externalService: externalServiceID,
254-
}),
259+
}).pipe(repeatWhen(obs => obs.pipe(delay(1500)))),
255260
[externalServiceID, queryExternalServiceSyncJobs]
256261
)
257262

@@ -272,7 +277,7 @@ const ExternalServiceSyncJobsList: React.FunctionComponent<ExternalServiceSyncJo
272277
pluralNoun="sync jobs"
273278
queryConnection={queryConnection}
274279
nodeComponent={ExternalServiceSyncJobNode}
275-
nodeComponentProps={{}}
280+
nodeComponentProps={{ onUpdate: updates }}
276281
hideSearch={true}
277282
noSummaryIfAllNodesVisible={true}
278283
history={history}
@@ -285,47 +290,83 @@ const ExternalServiceSyncJobsList: React.FunctionComponent<ExternalServiceSyncJo
285290

286291
interface ExternalServiceSyncJobNodeProps {
287292
node: ExternalServiceSyncJobListFields
293+
onUpdate: Subject<void>
288294
}
289295

290-
const ExternalServiceSyncJobNode: React.FunctionComponent<ExternalServiceSyncJobNodeProps> = ({ node }) => (
291-
<li className="list-group-item py-3">
292-
<div className="d-flex align-items-center justify-content-between">
293-
<div className="flex-shrink-0 mr-2">
294-
<Badge>{node.state}</Badge>
295-
</div>
296-
<div className="flex-shrink-0">
297-
{node.startedAt && (
298-
<>
299-
{node.finishedAt === null && <>Running since </>}
300-
{node.finishedAt !== null && <>Ran for </>}
301-
<Duration
302-
start={node.startedAt}
303-
end={node.finishedAt ?? undefined}
304-
stableWidth={false}
305-
className="d-inline"
306-
/>
307-
</>
308-
)}
309-
</div>
310-
<div className="text-right flex-grow-1">
311-
<div>
312-
{node.startedAt === null && 'Not started yet'}
313-
{node.startedAt !== null && (
314-
<>
315-
Started <Timestamp date={node.startedAt} />
316-
</>
317-
)}
296+
const ExternalServiceSyncJobNode: React.FunctionComponent<ExternalServiceSyncJobNodeProps> = ({ node, onUpdate }) => {
297+
const [
298+
cancelExternalServiceSync,
299+
{ error: cancelSyncJobError, loading: cancelSyncJobLoading },
300+
] = useCancelExternalServiceSync()
301+
302+
const cancelJob = useCallback(
303+
() =>
304+
cancelExternalServiceSync({ variables: { id: node.id } }).then(() => {
305+
onUpdate.next()
306+
// Optimistically set state.
307+
node.state = ExternalServiceSyncJobState.CANCELING
308+
}),
309+
[cancelExternalServiceSync, node, onUpdate]
310+
)
311+
312+
return (
313+
<li className="list-group-item py-3">
314+
<div className="d-flex align-items-center justify-content-between">
315+
<div className="flex-shrink-0 mr-2">
316+
<Badge>{node.state}</Badge>
318317
</div>
319-
<div>
320-
{node.finishedAt === null && 'Not finished yet'}
321-
{node.finishedAt !== null && (
318+
<div className="flex-shrink-0 flex-grow-1 mr-2">
319+
{node.startedAt && (
322320
<>
323-
Finished <Timestamp date={node.finishedAt} />
321+
{node.finishedAt === null && <>Running since </>}
322+
{node.finishedAt !== null && <>Ran for </>}
323+
<Duration
324+
start={node.startedAt}
325+
end={node.finishedAt ?? undefined}
326+
stableWidth={false}
327+
className="d-inline"
328+
/>
329+
{cancelSyncJobError && <ErrorAlert error={cancelSyncJobError} />}
324330
</>
325331
)}
326332
</div>
333+
{[
334+
ExternalServiceSyncJobState.QUEUED,
335+
ExternalServiceSyncJobState.PROCESSING,
336+
ExternalServiceSyncJobState.CANCELING,
337+
].includes(node.state) && (
338+
<LoaderButton
339+
label="Cancel"
340+
alwaysShowLabel={true}
341+
variant="danger"
342+
outline={true}
343+
size="sm"
344+
onClick={cancelJob}
345+
loading={cancelSyncJobLoading || node.state === ExternalServiceSyncJobState.CANCELING}
346+
disabled={cancelSyncJobLoading || node.state === ExternalServiceSyncJobState.CANCELING}
347+
className={styles.cancelButton}
348+
/>
349+
)}
350+
<div className="text-right flex-shrink-0">
351+
<div>
352+
{node.startedAt === null && 'Not started yet'}
353+
{node.startedAt !== null && (
354+
<>
355+
Started <Timestamp date={node.startedAt} />
356+
</>
357+
)}
358+
</div>
359+
<div>
360+
{node.finishedAt === null && 'Not finished yet'}
361+
{node.finishedAt !== null && (
362+
<>
363+
Finished <Timestamp date={node.finishedAt} />
364+
</>
365+
)}
366+
</div>
367+
</div>
327368
</div>
328-
</div>
329-
{node.failureMessage && <ErrorAlert error={node.failureMessage} className="mt-2 mb-0" />}
330-
</li>
331-
)
369+
{node.failureMessage && <ErrorAlert error={node.failureMessage} className="mt-2 mb-0" />}
370+
</li>
371+
)
372+
}

client/web/src/components/externalServices/backend.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import {
2525
ExternalServiceSyncJobsVariables,
2626
ExternalServiceSyncJobConnectionFields,
2727
ExternalServiceSyncJobsResult,
28+
CancelExternalServiceSyncVariables,
29+
CancelExternalServiceSyncResult,
2830
} from '../../graphql-operations'
2931

3032
export const externalServiceFragment = gql`
@@ -262,6 +264,23 @@ export function useSyncExternalService(): MutationTuple<SyncExternalServiceResul
262264
return useMutation<SyncExternalServiceResult, SyncExternalServiceVariables>(SYNC_EXTERNAL_SERVICE)
263265
}
264266

267+
export const CANCEL_EXTERNAL_SERVICE_SYNC = gql`
268+
mutation CancelExternalServiceSync($id: ID!) {
269+
cancelExternalServiceSync(id: $id) {
270+
alwaysNil
271+
}
272+
}
273+
`
274+
275+
export function useCancelExternalServiceSync(): MutationTuple<
276+
CancelExternalServiceSyncResult,
277+
CancelExternalServiceSyncVariables
278+
> {
279+
return useMutation<CancelExternalServiceSyncResult, CancelExternalServiceSyncVariables>(
280+
CANCEL_EXTERNAL_SERVICE_SYNC
281+
)
282+
}
283+
265284
export const EXTERNAL_SERVICE_SYNC_JOBS = gql`
266285
query ExternalServiceSyncJobs($first: Int, $externalService: ID!) {
267286
node(id: $externalService) {

client/web/src/components/time/Duration.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export const Duration: React.FunctionComponent<React.PropsWithChildren<DurationP
5757
}, [end])
5858

5959
return (
60-
<div className={classNames({ [styles.stableWidth]: stableWidth }, className)} {...props}>
60+
<div className={classNames('chromatic-ignore', { [styles.stableWidth]: stableWidth }, className)} {...props}>
6161
{stableWidth && (
6262
// Set the width of the parent with a filler block of full-width digits,
6363
// to prevent layout shift if the time changes.

cmd/frontend/graphqlbackend/external_service.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,11 +324,14 @@ func (r *externalServiceSyncJobResolver) ID() graphql.ID {
324324
}
325325

326326
func (r *externalServiceSyncJobResolver) State() string {
327+
if r.job.Cancel && r.job.State == "processing" {
328+
return "CANCELING"
329+
}
327330
return strings.ToUpper(r.job.State)
328331
}
329332

330333
func (r *externalServiceSyncJobResolver) FailureMessage() *string {
331-
if r.job.FailureMessage == "" {
334+
if r.job.FailureMessage == "" || r.job.Cancel {
332335
return nil
333336
}
334337

cmd/frontend/graphqlbackend/external_services.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,3 +473,39 @@ func (r *schemaResolver) SyncExternalService(ctx context.Context, args *syncExte
473473

474474
return &EmptyResponse{}, nil
475475
}
476+
477+
type cancelExternalServiceSyncArgs struct {
478+
ID graphql.ID
479+
}
480+
481+
func (r *schemaResolver) CancelExternalServiceSync(ctx context.Context, args *cancelExternalServiceSyncArgs) (*EmptyResponse, error) {
482+
start := time.Now()
483+
var err error
484+
var namespaceUserID, namespaceOrgID int32
485+
defer reportExternalServiceDuration(start, Update, &err, &namespaceUserID, &namespaceOrgID)
486+
487+
id, err := unmarshalExternalServiceSyncJobID(args.ID)
488+
if err != nil {
489+
return nil, err
490+
}
491+
492+
esj, err := r.db.ExternalServices().GetSyncJobByID(ctx, id)
493+
if err != nil {
494+
return nil, err
495+
}
496+
es, err := r.db.ExternalServices().GetByID(ctx, esj.ExternalServiceID)
497+
if err != nil {
498+
return nil, err
499+
}
500+
501+
// 🚨 SECURITY: check access to external service.
502+
if err = backend.CheckExternalServiceAccess(ctx, r.db, es.NamespaceUserID, es.NamespaceOrgID); err != nil {
503+
return nil, err
504+
}
505+
506+
if err := r.db.ExternalServices().CancelSyncJob(ctx, esj.ID); err != nil {
507+
return nil, err
508+
}
509+
510+
return &EmptyResponse{}, nil
511+
}

0 commit comments

Comments
 (0)