Skip to content
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.

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

Allow specifying failure policies #482

Merged
merged 56 commits into from
Nov 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
6a8e381
Define an interface for FailurePolicy
merlinnot Jun 21, 2019
282da1b
Extract functions config to avoid dependency cycles, assign failure p…
merlinnot Jun 21, 2019
33137a6
Add a changelog entry
merlinnot Jun 21, 2019
facd69c
Update dependencies, minor version bump
merlinnot Jun 21, 2019
819a564
Reformat
merlinnot Jun 21, 2019
f42584c
Fix a typo: allows -> allow
merlinnot Jun 21, 2019
0c554db
Avoid abbreviations to improve readability
merlinnot Jun 21, 2019
c633a78
Rename remaining Opts to Options
merlinnot Jun 24, 2019
568113c
Add tests for specifying failure policy
merlinnot Jun 24, 2019
993907f
Reformat
merlinnot Jun 24, 2019
acb761d
Merge branch 'master' into fix-issue-425
merlinnot Jun 25, 2019
b1344df
Merge branch 'master' into fix-issue-425
merlinnot Jun 25, 2019
a23beff
Merge branch 'master' into fix-issue-425
merlinnot Jul 2, 2019
3e46d65
Change format of an entry in the changelog
merlinnot Jul 3, 2019
b0ad579
Revert version bump
merlinnot Jul 3, 2019
6893b35
Extract configuration to break dependency cycle
merlinnot Jul 3, 2019
ae767a6
Merge branch 'master' into extract-config
merlinnot Jul 3, 2019
3fc81ff
Merge branch 'master' into extract-config
merlinnot Jul 3, 2019
f28e5bf
Add comments to Schedule and ScheduleRetryConfig
merlinnot Jul 3, 2019
22b6d0b
Stricten regions definition
merlinnot Jul 3, 2019
85d1329
Fix tests broken due to strict typings
merlinnot Jul 3, 2019
3ee8a0a
More strict types for regions - reverse order
merlinnot Jul 3, 2019
0cbe15a
Reformat
merlinnot Jul 3, 2019
a9b0929
Merge branch 'extract-config' into fix-issue-425
merlinnot Jul 3, 2019
61ebd4a
Merge branch 'master' into fix-issue-425
merlinnot Jul 3, 2019
a95e7de
Remove unused import
merlinnot Jul 3, 2019
2fced88
Merge branch 'master' into fix-issue-425
merlinnot Jul 9, 2019
3695897
Merge branch 'master' into fix-issue-425
merlinnot Jul 10, 2019
b1c6737
Conform with npm formatting
merlinnot Jul 10, 2019
b2c9b7f
Reintroduce unused variable to minimize diff size
merlinnot Jul 10, 2019
3a0dd5a
Import lodash using _ exclusively
merlinnot Jul 10, 2019
85eb8a0
Kepp @hidden tags in one line if no other JSDoc is present
merlinnot Jul 10, 2019
1c42e0e
Fix a typo - sentence ending with a comma
merlinnot Jul 10, 2019
8873af6
Merge branch 'master' into fix-issue-425
merlinnot Jul 11, 2019
6809ae5
Merge branch 'master' into fix-issue-425
merlinnot Jul 11, 2019
540fb95
Merge branch 'master' into fix-issue-425
merlinnot Jul 12, 2019
dbed779
Merge branch 'master' into fix-issue-425
merlinnot Jul 12, 2019
d741c04
Merge branch 'master' into fix-issue-425
merlinnot Jul 16, 2019
4cb2f28
Wrap JSDoc as specified in Google JavaScript Style Guide
merlinnot Jul 16, 2019
cd6b147
Move memory lookup table to functions-configuration and stricten type…
merlinnot Jul 17, 2019
06002e1
Move default failure policy to functions-configuration
merlinnot Jul 17, 2019
7b56f21
Separate standarization of options from construction of the options o…
merlinnot Jul 17, 2019
3a7c851
Rephrase failure policy description
merlinnot Jul 17, 2019
2e35c26
Rephrase description of memory and timeoutSeconds options
merlinnot Jul 17, 2019
4059b00
Simplify tests for invalid failure policies
merlinnot Jul 17, 2019
86b08cf
Merge branch 'master' into fix-issue-425
merlinnot Jul 17, 2019
2b95df9
Align public description of failure policies with a documentation of …
merlinnot Jul 17, 2019
d0e1a13
Merge branch 'fix-issue-425' of github.com:merlinnot/firebase-functio…
merlinnot Jul 17, 2019
006e11c
Merge branch 'master' into fix-issue-425
merlinnot Jul 17, 2019
31952e2
Add a missing dot in the Changelog
merlinnot Jul 18, 2019
668c02f
Minor stylistic changes to the documentation
merlinnot Jul 18, 2019
a89656c
Merge branch 'master' into fix-issue-425
merlinnot Jul 18, 2019
ac09c18
Make a test case more understandable
merlinnot Jul 19, 2019
7d3b4de
Merge branch 'master' into fix-issue-425
merlinnot Jul 19, 2019
f352933
Merge branch 'master' into fix-issue-425
samtstern Nov 25, 2019
1b6281f
Reformat
merlinnot Nov 25, 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
1 change: 1 addition & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
feature - Allow specifying retry policies for event triggered functions.
42 changes: 38 additions & 4 deletions spec/function-builder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,29 @@ describe('FunctionBuilder', () => {
it('should allow valid runtime options to be set', () => {
const fn = functions
.runWith({
timeoutSeconds: 90,
failurePolicy: { retry: {} },
memory: '256MB',
timeoutSeconds: 90,
})
.auth.user()
.onCreate((user) => user);

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

it("should apply a default failure policy if it's aliased with `true`", () => {
const fn = functions
.runWith({
failurePolicy: true,
memory: '256MB',
timeoutSeconds: 90,
})
.auth.user()
.onCreate((user) => user);

expect(fn.__trigger.failurePolicy).to.deep.equal({ retry: {} });
});

it('should allow both supported region and valid runtime options to be set', () => {
Expand Down Expand Up @@ -132,7 +147,26 @@ describe('FunctionBuilder', () => {
functions
.region('asia-northeast1')
.runWith({ timeoutSeconds: 600, memory: '256MB' });
}).to.throw(Error, 'TimeoutSeconds');
}).to.throw(Error, 'RuntimeOptions.timeoutSeconds');
});

it('should throw an error if user chooses a failurePolicy which is neither an object nor a boolean', () => {
merlinnot marked this conversation as resolved.
Show resolved Hide resolved
expect(() =>
functions.runWith({
failurePolicy: (1234 as unknown) as functions.RuntimeOptions['failurePolicy'],
})
).to.throw(
Error,
'RuntimeOptions.failurePolicy must be a boolean or an object'
);
});

it('should throw an error if user chooses a failurePolicy.retry which is not an object', () => {
expect(() =>
functions.runWith({
failurePolicy: { retry: (1234 as unknown) as object },
})
).to.throw(Error, 'RuntimeOptions.failurePolicy.retry');
});

it('should throw an error if user chooses an invalid memory allocation', () => {
Expand All @@ -154,13 +188,13 @@ describe('FunctionBuilder', () => {
return functions.runWith({
timeoutSeconds: 1000000,
} as any);
}).to.throw(Error, 'TimeoutSeconds');
}).to.throw(Error, 'RuntimeOptions.timeoutSeconds');

expect(() => {
return functions.region('asia-east2').runWith({
timeoutSeconds: 1000000,
} as any);
}).to.throw(Error, 'TimeoutSeconds');
}).to.throw(Error, 'RuntimeOptions.timeoutSeconds');
});

it('should throw an error if user chooses an invalid region', () => {
Expand Down
4 changes: 1 addition & 3 deletions spec/providers/auth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,7 @@ describe('Auth Functions', () => {
});

describe('#onDelete', () => {
const cloudFunctionDelete: CloudFunction<
firebase.auth.UserRecord
> = functions.handler.auth.user.onDelete(
const cloudFunctionDelete: CloudFunction<firebase.auth.UserRecord> = functions.handler.auth.user.onDelete(
(data: firebase.auth.UserRecord) => data
);

Expand Down
73 changes: 46 additions & 27 deletions src/cloud-functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,13 @@

import { Request, Response } from 'express';
import * as _ from 'lodash';
import { DeploymentOptions, Schedule } from './function-configuration';
import {
DEFAULT_FAILURE_POLICY,
DeploymentOptions,
FailurePolicy,
MEMORY_LOOKUP,
Schedule,
} from './function-configuration';
export { Request, Response };

/** @hidden */
Expand Down Expand Up @@ -202,6 +208,7 @@ export namespace Change {
if (json.fieldMask) {
before = applyFieldMask(before, json.after, json.fieldMask);
}

return Change.fromObjects(
customizer(before || {}),
customizer(json.after || {})
Expand All @@ -216,14 +223,16 @@ export namespace Change {
) {
const before = _.assign({}, after);
const masks = fieldMask.split(',');
_.forEach(masks, (mask) => {

masks.forEach((mask) => {
const val = _.get(sparseBefore, mask);
if (typeof val === 'undefined') {
_.unset(before, mask);
} else {
_.set(before, mask, val);
}
});

return before;
}
}
Expand Down Expand Up @@ -253,6 +262,7 @@ export interface TriggerAnnotated {
resource: string;
service: string;
};
failurePolicy?: FailurePolicy;
httpsTrigger?: {};
labels?: { [key: string]: string };
regions?: string[];
Expand Down Expand Up @@ -312,6 +322,40 @@ export interface MakeCloudFunctionArgs<EventData> {
triggerResource: () => string;
}

/** @hidden */
export function optionsToTrigger({
failurePolicy: failurePolicyOrAlias,
memory,
regions,
schedule,
timeoutSeconds,
}: DeploymentOptions): TriggerAnnotated['__trigger'] {
/*
* FailurePolicy can be aliased with a boolean value in the public API.
* Convert aliases `true` and `false` to a standardized interface.
*/
const failurePolicy: FailurePolicy | undefined =
failurePolicyOrAlias === false
? undefined
: failurePolicyOrAlias === true
? DEFAULT_FAILURE_POLICY
: failurePolicyOrAlias;

const availableMemoryMb: number | undefined =
memory === undefined ? undefined : MEMORY_LOOKUP[memory];

const timeout: string | undefined =
timeoutSeconds === undefined ? undefined : `${timeoutSeconds}s`;

return {
thechenky marked this conversation as resolved.
Show resolved Hide resolved
...(failurePolicy === undefined ? {} : { failurePolicy }),
...(availableMemoryMb === undefined ? {} : { availableMemoryMb }),
...(regions === undefined ? {} : { regions }),
...(schedule === undefined ? {} : { schedule }),
...(timeout === undefined ? {} : { timeout }),
};
}

/** @hidden */
export function makeCloudFunction<EventData>({
after = () => {},
Expand Down Expand Up @@ -463,28 +507,3 @@ function _detectAuthType(event: Event) {
}
return 'UNAUTHENTICATED';
}

/** @hidden */
export function optionsToTrigger(options: DeploymentOptions) {
const trigger: any = {};
if (options.regions) {
trigger.regions = options.regions;
}
if (options.timeoutSeconds) {
trigger.timeout = options.timeoutSeconds.toString() + 's';
}
if (options.memory) {
const memoryLookup = {
'128MB': 128,
'256MB': 256,
'512MB': 512,
'1GB': 1024,
'2GB': 2048,
};
trigger.availableMemoryMb = _.get(memoryLookup, options.memory);
}
if (options.schedule) {
trigger.schedule = options.schedule;
}
return trigger;
}
Loading