Skip to content

adds retry option to event triggered functions #370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

adds retry option to event triggered functions #370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5485093
adds retry option to event triggered functions
joehan Jan 3, 2019
d71acc9
fixing number of args in jsdoc
joehan Jan 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions spec/cloud-functions.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,43 @@ describe('makeCloudFunction', () => {
});
});

it('should put a __trigger that includes failurePolicy on the returned CloudFunction if opts.retry = true', () => {
let cf = makeCloudFunction({
provider: 'mock.provider',
eventType: 'mock.event',
service: 'service',
triggerResource: () => 'resource',
handler: () => null,
opts: { retry: true },
});
expect(cf.__trigger).to.deep.equal({
eventTrigger: {
eventType: 'mock.provider.mock.event',
resource: 'resource',
service: 'service',
failurePolicy: { retry: {} },
},
});
});

it('should put a __trigger that does not include failurePolicy on the returned CloudFunction if opts.retry = false', () => {
let cf = makeCloudFunction({
provider: 'mock.provider',
eventType: 'mock.event',
service: 'service',
triggerResource: () => 'resource',
handler: () => null,
opts: { retry: false },
});
expect(cf.__trigger).to.deep.equal({
eventTrigger: {
eventType: 'mock.provider.mock.event',
resource: 'resource',
service: 'service',
},
});
});

it('should have legacy event type in __trigger if provided', () => {
let cf = makeCloudFunction(cloudFunctionArgs);
expect(cf.__trigger).to.deep.equal({
Expand Down
6 changes: 6 additions & 0 deletions spec/function-builder.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,14 @@ describe('FunctionBuilder', () => {
.runWith({
timeoutSeconds: 90,
memory: '256MB',
retry: true,
})
.auth.user()
.onCreate(user => user);

expect(fn.__trigger.availableMemoryMb).to.deep.equal(256);
expect(fn.__trigger.timeout).to.deep.equal('90s');
expect(fn.__trigger.eventTrigger.failurePolicy).to.deep.equal({ retry: {} });
});

it('should allow both region and runtime options to be set', () => {
Expand All @@ -73,20 +75,23 @@ describe('FunctionBuilder', () => {
.runWith({
timeoutSeconds: 90,
memory: '256MB',
retry: true,
})
.auth.user()
.onCreate(user => user);

expect(fn.__trigger.regions).to.deep.equal(['my-region']);
expect(fn.__trigger.availableMemoryMb).to.deep.equal(256);
expect(fn.__trigger.timeout).to.deep.equal('90s');
expect(fn.__trigger.eventTrigger.failurePolicy).to.deep.equal({ retry: {} });
});

it('should allow both region and runtime options to be set (reverse order)', () => {
let fn = functions
.runWith({
timeoutSeconds: 90,
memory: '256MB',
retry: true,
})
.region('my-region')
.auth.user()
Expand All @@ -95,6 +100,7 @@ describe('FunctionBuilder', () => {
expect(fn.__trigger.regions).to.deep.equal(['my-region']);
expect(fn.__trigger.availableMemoryMb).to.deep.equal(256);
expect(fn.__trigger.timeout).to.deep.equal('90s');
expect(fn.__trigger.eventTrigger.failurePolicy).to.deep.equal({ retry: {} });
});

it('should throw an error if user chooses an unsupported memory allocation', () => {
Expand Down
5 changes: 4 additions & 1 deletion src/cloud-functions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ export interface TriggerAnnotated {
httpsTrigger?: {};
eventTrigger?: {
eventType: string;
failurePolicy?: object;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious as to why failurePolicy is inside the eventTrigger and not outside like the other options like timeout and memory? was that because it does not apply to http functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is per the Google cloud docs - since it only applies to event-triggered functions, its a property of EventTrigger: https://cloud.google.com/functions/docs/reference/rest/v1/projects.locations.functions#EventTrigger

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, makes sense

resource: string;
service: string;
};
Expand Down Expand Up @@ -303,7 +304,9 @@ export function makeCloudFunction<EventData>({
service,
},
});

if (opts.retry) {
trigger.eventTrigger.failurePolicy = { retry: {} };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this propagate the retry boolean instead of creating an empty object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope - strangely enough, google cloud expects an empty object of type Retry here: https://cloud.google.com/functions/docs/reference/rest/v1/projects.locations.functions#EventTrigger

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noted, interesting

}
return trigger;
},
});
Expand Down
11 changes: 9 additions & 2 deletions src/function-builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,17 @@ export function region(region: string) {

/**
* Configure runtime options for the function.
* @param runtimeOptions Object with 2 optional fields:
* @param runtimeOptions Object with 3 optional fields:
* 1. `timeoutSeconds`: timeout for the function in seconds.
* 2. `memory`: amount of memory to allocate to the function,
* possible values are: '128MB', '256MB', '512MB', '1GB', and '2GB'.
* 3. `retry`: whether to retry the function on failure,
* not compatible with http functions
*/
export function runWith(runtimeOptions: {
timeoutSeconds?: number;
memory?: '128MB' | '256MB' | '512MB' | '1GB' | '2GB';
retry?: boolean;
}) {
if (
runtimeOptions.memory &&
Expand All @@ -72,6 +75,7 @@ export interface DeploymentOptions {
regions?: string[];
timeoutSeconds?: number;
memory?: string;
retry?: boolean;
}

export class FunctionBuilder {
Expand All @@ -89,14 +93,17 @@ export class FunctionBuilder {

/**
* Configure runtime options for the function.
* @param runtimeOptions Object with 2 optional fields:
* @param runtimeOptions Object with 3 optional fields:
* 1. timeoutSeconds: timeout for the function in seconds.
* 2. memory: amount of memory to allocate to the function, possible values are:
* '128MB', '256MB', '512MB', '1GB', and '2GB'.
* 3. retry: whether to retry the function on failure,
* not compatible with http functions
*/
runWith = (runtimeOptions: {
timeoutSeconds?: number;
memory?: '128MB' | '256MB' | '512MB' | '1GB' | '2GB';
retry?: boolean;
}) => {
if (
runtimeOptions.memory &&
Expand Down