Skip to content

Commit

Permalink
fix(triggers): Poll on execution id instead of event id (#7067)
Browse files Browse the repository at this point in the history
The V2 manual trigger endpoint only returned an event id,
so deck had to poll for the appearance of a new trigger by
polling on the event id.

Now that the endpoint also returns the execution id, we can
poll on the execution id which should be significantly less
expensive (for both orca and the redis/sql back-end).
  • Loading branch information
ezimanyi committed May 30, 2019
1 parent 6af93e3 commit b142789
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { IPipeline } from 'core/domain/IPipeline';

export interface ITriggerPipelineResponse {
eventId: string;
ref: string;
}
export class PipelineConfigService {
private static configViewStateCache = ViewStateCache.createCache('pipelineConfig', { version: 2 });
Expand Down Expand Up @@ -107,7 +108,7 @@ export class PipelineConfigService {
.data(body)
.post()
.then((result: ITriggerPipelineResponse) => {
return result.eventId;
return result.ref.split('/').pop();
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -451,21 +451,18 @@ describe('Service: executionService', () => {

describe('waitUntilTriggeredPipelineAppears', () => {
const applicationName = 'deck';
const pipelineName = 'pipeline';
const application: Application = { name: applicationName, executions: { refresh: () => $q.when(null) } } as any;
const eventId = 'abc';
const url =
[SETTINGS.gateUrl, 'applications', 'deck', 'executions', 'search'].join('/') +
'?eventId=abc&pipelineName=pipeline';
const pipelineId = '01DC2VMFBZ5PFW5G6SMKWW5CZC';
const url = [SETTINGS.gateUrl, 'pipelines', pipelineId].join('/');
const execution: any = {}; // Stub execution

it('resolves when the pipeline exists', () => {
let succeeded = false;

$httpBackend.expectGET(url).respond(200, [execution]);
$httpBackend.expectGET(url).respond(200, execution);

executionService
.waitUntilTriggeredPipelineAppears(application, pipelineName, eventId)
.waitUntilTriggeredPipelineAppears(application, pipelineId)
.promise.then(() => (succeeded = true));

expect(succeeded).toBe(false);
Expand All @@ -477,10 +474,10 @@ describe('Service: executionService', () => {
it('does not resolve when the pipeline does not exist', () => {
let succeeded = false;

$httpBackend.expectGET(url).respond(200, []);
$httpBackend.expectGET(url).respond(404, null);

executionService
.waitUntilTriggeredPipelineAppears(application, pipelineName, eventId)
.waitUntilTriggeredPipelineAppears(application, pipelineId)
.promise.then(() => (succeeded = true));

expect(succeeded).toBe(false);
Expand All @@ -492,10 +489,10 @@ describe('Service: executionService', () => {
it('resolves when the pipeline exists on a later poll', () => {
let succeeded = false;

$httpBackend.expectGET(url).respond(200, []);
$httpBackend.expectGET(url).respond(404, null);

executionService
.waitUntilTriggeredPipelineAppears(application, pipelineName, eventId)
.waitUntilTriggeredPipelineAppears(application, pipelineId)
.promise.then(() => (succeeded = true));

expect(succeeded).toBe(false);
Expand All @@ -505,7 +502,7 @@ describe('Service: executionService', () => {
expect(succeeded).toBe(false);

// return success on the second GET request
$httpBackend.expectGET(url).respond(200, [execution]);
$httpBackend.expectGET(url).respond(200, execution);
timeout.flush();
$httpBackend.flush();

Expand Down
24 changes: 3 additions & 21 deletions app/scripts/modules/core/src/pipeline/service/execution.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { DebugWindow } from 'core/utils/consoleDebug';
import { IPipeline } from 'core/domain/IPipeline';
import { ISortFilter } from 'core/filterModel';
import { ExecutionState } from 'core/state';
import { Error } from 'tslint/lib/error';
import { IRetryablePromise, retryablePromise } from 'core/utils/retryablePromise';
import { ReactInjector } from 'core/reactShims';
import { PipelineConfigService } from 'core/pipeline/config/services/PipelineConfigService';
Expand Down Expand Up @@ -120,21 +119,6 @@ export class ExecutionService {
});
}

public getExecutionByEventId(application: string, pipelineName: string, eventId: string): IPromise<IExecution> {
return API.all('applications', application, 'executions', 'search')
.get({ pipelineName, eventId })
.then((data: IExecution[]) => {
if (data.length > 0) {
const execution = data[0];
execution.hydrated = true;
this.cleanExecutionForDiffing(execution);
return execution;
} else {
throw new Error('No execution found for event id: ' + eventId);
}
});
}

public transformExecution(application: Application, execution: IExecution): void {
ExecutionsTransformer.transformExecution(application, execution);
}
Expand Down Expand Up @@ -240,17 +224,15 @@ export class ExecutionService {
public startAndMonitorPipeline(app: Application, pipeline: string, trigger: any): IPromise<IRetryablePromise<void>> {
const { executionService } = ReactInjector;
return PipelineConfigService.triggerPipeline(app.name, pipeline, trigger).then(triggerResult =>
executionService.waitUntilTriggeredPipelineAppears(app, pipeline, triggerResult),
executionService.waitUntilTriggeredPipelineAppears(app, triggerResult),
);
}

public waitUntilTriggeredPipelineAppears(
application: Application,
pipelineName: string,
eventId: string,
triggeredPipelineId: string,
): IRetryablePromise<any> {
const closure = () =>
this.getExecutionByEventId(application.name, pipelineName, eventId).then(() => application.executions.refresh());
const closure = () => this.getExecution(triggeredPipelineId).then(() => application.executions.refresh());
return retryablePromise(closure, 1000, 10);
}

Expand Down

0 comments on commit b142789

Please sign in to comment.