Skip to content

Commit

Permalink
Revert "Allow specifying failure policies" (#623)
Browse files Browse the repository at this point in the history
* Revert "Allow specifying failure policies (#482)
  • Loading branch information
tinaliang authored Feb 5, 2020
1 parent ae90279 commit d9fc8a6
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 211 deletions.
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

0 comments on commit d9fc8a6

Please sign in to comment.