Skip to content

Revert "Allow specifying failure policies" #623

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

Merged
merged 4 commits into from
Feb 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 0 additions & 1 deletion changelog.txt
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@
feature - Allow specifying retry policies for event triggered functions.
42 changes: 4 additions & 38 deletions spec/function-builder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,29 +79,14 @@ describe('FunctionBuilder', () => {
it('should allow valid runtime options to be set', () => {
const fn = functions
.runWith({
failurePolicy: { retry: {} },
memory: '256MB',
timeoutSeconds: 90,
memory: '256MB',
})
.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 @@ -147,26 +132,7 @@ describe('FunctionBuilder', () => {
functions
.region('asia-northeast1')
.runWith({ timeoutSeconds: 600, memory: '256MB' });
}).to.throw(Error, 'RuntimeOptions.timeoutSeconds');
});

it('should throw an error if user chooses a failurePolicy which is neither an object nor a boolean', () => {
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');
}).to.throw(Error, 'TimeoutSeconds');
});

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

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

it('should throw an error if user chooses an invalid region', () => {
Expand Down
73 changes: 27 additions & 46 deletions src/cloud-functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,7 @@

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

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

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

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

return before;
}
}
Expand Down Expand Up @@ -262,7 +253,6 @@ export interface TriggerAnnotated {
resource: string;
service: string;
};
failurePolicy?: FailurePolicy;
httpsTrigger?: {};
labels?: { [key: string]: string };
regions?: string[];
Expand Down Expand Up @@ -322,40 +312,6 @@ 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 {
...(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 @@ -507,3 +463,28 @@ 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