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

Fix schedule function deployment #1305

Merged
merged 6 commits into from
Nov 14, 2022
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: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
- Deprecate typoed function name lessThanorEqualTo (#1284)
- Fix a bug where supplying preserveExternalChanges to scheduled functions would cause deployment failure (#1305).
97 changes: 96 additions & 1 deletion spec/runtime/manifest.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
import { expect } from "chai";
import { stackToWire, ManifestStack } from "../../src/runtime/manifest";
import {
stackToWire,
ManifestStack,
initV2ScheduleTrigger,
initV1ScheduleTrigger,
initTaskQueueTrigger,
} from "../../src/runtime/manifest";
import { RESET_VALUE } from "../../src/common/options";
import * as params from "../../src/params";
import * as optsv2 from "../../src/v2/options";
import * as v1 from "../../src/v1";
import { DeploymentOptions } from "../../src/v1";

describe("stackToWire", () => {
afterEach(() => {
Expand Down Expand Up @@ -168,3 +176,90 @@ describe("stackToWire", () => {
expect(stackToWire(stack)).to.deep.equal(expected);
});
});

describe("initTaskQueueTrigger", () => {
it("should init a taskQueueTrigger without preserveExternalChanges", () => {
const tt = initTaskQueueTrigger();

expect(tt).to.deep.eq({
retryConfig: {
maxAttempts: RESET_VALUE,
maxDoublings: RESET_VALUE,
maxBackoffSeconds: RESET_VALUE,
maxRetrySeconds: RESET_VALUE,
minBackoffSeconds: RESET_VALUE,
},
rateLimits: {
maxConcurrentDispatches: RESET_VALUE,
maxDispatchesPerSecond: RESET_VALUE,
},
});
});

it("should init a taskQueueTrigger with preserveExternalChanges", () => {
const opts: DeploymentOptions = { preserveExternalChanges: true };

const tt = initTaskQueueTrigger(opts);

expect(tt).to.deep.eq({
rateLimits: {},
retryConfig: {},
});
});
});

describe("initScheduleTrigger", () => {
it("should init a v1 scheduleTrigger without preserveExternalChanges", () => {
const st = initV1ScheduleTrigger("every 30 minutes");

expect(st).to.deep.eq({
schedule: "every 30 minutes",
timeZone: RESET_VALUE,
retryConfig: {
retryCount: RESET_VALUE,
maxDoublings: RESET_VALUE,
maxRetryDuration: RESET_VALUE,
minBackoffDuration: RESET_VALUE,
maxBackoffDuration: RESET_VALUE,
},
});
});

it("should init a v1 scheduleTrigger with preserveExternalChanges", () => {
const opts: DeploymentOptions = { preserveExternalChanges: true };

const st = initV1ScheduleTrigger("every 30 minutes", opts);

expect(st).to.deep.eq({
schedule: "every 30 minutes",
retryConfig: {},
});
});

it("should init a v2 scheduleTrigger without preserveExternalChanges", () => {
const st = initV2ScheduleTrigger("every 30 minutes");

expect(st).to.deep.eq({
schedule: "every 30 minutes",
timeZone: RESET_VALUE,
retryConfig: {
retryCount: RESET_VALUE,
maxDoublings: RESET_VALUE,
maxRetrySeconds: RESET_VALUE,
minBackoffSeconds: RESET_VALUE,
maxBackoffSeconds: RESET_VALUE,
},
});
});

it("should init a v2 scheduleTrigger with preserveExternalChanges", () => {
const opts: DeploymentOptions = { preserveExternalChanges: true };

const st = initV2ScheduleTrigger("every 30 minutes", opts);

expect(st).to.deep.eq({
schedule: "every 30 minutes",
retryConfig: {},
});
});
});
29 changes: 29 additions & 0 deletions spec/v1/cloud-functions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,35 @@ describe("makeCloudFunction", () => {
});
});

it("should setup a scheduleTrigger in __endpoint given a schedule and preserveExternalChanges", () => {
const schedule = {
schedule: "every 5 minutes",
retryConfig: { retryCount: 3 },
timeZone: "America/New_York",
};
const cf = makeCloudFunction({
provider: "mock.provider",
eventType: "mock.event",
service: "service",
triggerResource: () => "resource",
handler: () => null,
options: {
schedule,
preserveExternalChanges: true,
},
});
expect(cf.__endpoint).to.deep.equal({
platform: "gcfv1",
scheduleTrigger: {
...schedule,
retryConfig: {
...schedule.retryConfig,
},
},
labels: {},
});
});

it("should construct the right context for event", () => {
const args: any = {
...cloudFunctionArgs,
Expand Down
26 changes: 26 additions & 0 deletions spec/v1/providers/pubsub.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,32 @@ describe("Pubsub Functions", () => {
expect(result.__endpoint.availableMemoryMb).to.deep.equal(256);
expect(result.__endpoint.timeoutSeconds).to.deep.equal(90);
});

it("should return an appropriate endpoint when called with preserveExternalChanges", () => {
const result = functions
.region("us-east1")
.runWith({
timeoutSeconds: 90,
memory: "256MB",
preserveExternalChanges: true,
})
.pubsub.schedule("every 5 minutes")
.timeZone("America/New_York")
.onRun(() => null);

expect(result.__endpoint).to.deep.eq({
platform: "gcfv1",
labels: {},
region: ["us-east1"],
availableMemoryMb: 256,
timeoutSeconds: 90,
scheduleTrigger: {
schedule: "every 5 minutes",
timeZone: "America/New_York",
retryConfig: {},
},
});
});
});
});

Expand Down
35 changes: 35 additions & 0 deletions spec/v1/providers/tasks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { MockRequest } from "../../fixtures/mockrequest";
import { runHandler } from "../../helper";
import { MINIMAL_V1_ENDPOINT } from "../../fixtures";
import { MINIMIAL_TASK_QUEUE_TRIGGER } from "./fixtures";
import { runWith } from "../../../src/v1";

describe("#onDispatch", () => {
it("should return a trigger/endpoint with appropriate values", () => {
Expand Down Expand Up @@ -67,6 +68,7 @@ describe("#onDispatch", () => {
...MINIMAL_V1_ENDPOINT,
platform: "gcfv1",
taskQueueTrigger: {
...MINIMIAL_TASK_QUEUE_TRIGGER,
rateLimits: {
maxConcurrentDispatches: 30,
maxDispatchesPerSecond: 40,
Expand All @@ -83,6 +85,35 @@ describe("#onDispatch", () => {
});
});

it("should return an endpoint with appropriate values with preserveExternalChanges set", () => {
const result = runWith({ preserveExternalChanges: true })
.tasks.taskQueue({
rateLimits: {
maxConcurrentDispatches: 30,
},
retryConfig: {
maxAttempts: 5,
maxRetrySeconds: 10,
},
invoker: "private",
})
.onDispatch(() => undefined);

expect(result.__endpoint).to.deep.equal({
platform: "gcfv1",
taskQueueTrigger: {
rateLimits: {
maxConcurrentDispatches: 30,
},
retryConfig: {
maxAttempts: 5,
maxRetrySeconds: 10,
},
invoker: ["private"],
},
});
});

it("should allow both region and runtime options to be set", () => {
const fn = functions
.region("us-east1")
Expand Down Expand Up @@ -114,6 +145,10 @@ describe("#onDispatch", () => {
...MINIMIAL_TASK_QUEUE_TRIGGER,
retryConfig: {
maxAttempts: 5,
maxBackoffSeconds: functions.RESET_VALUE,
maxDoublings: functions.RESET_VALUE,
maxRetrySeconds: functions.RESET_VALUE,
minBackoffSeconds: functions.RESET_VALUE,
},
},
});
Expand Down
44 changes: 39 additions & 5 deletions spec/v2/providers/scheduler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,13 @@ describe("schedule", () => {
expect(schedule.getOpts(options)).to.deep.eq({
schedule: "* * * * *",
timeZone: "utc",
retryCount: 3,
maxRetrySeconds: 1,
minBackoffSeconds: 2,
maxBackoffSeconds: 3,
maxDoublings: 4,
retryConfig: {
retryCount: 3,
maxRetrySeconds: 1,
minBackoffSeconds: 2,
maxBackoffSeconds: 3,
maxDoublings: 4,
},
opts: {
...options,
memory: "128MiB",
Expand Down Expand Up @@ -139,6 +141,38 @@ describe("schedule", () => {
]);
});

it("should create a schedule function with preserveExternalChanges", () => {
const schfn = schedule.onSchedule(
{
schedule: "* * * * *",
preserveExternalChanges: true,
},
() => console.log(1)
);

expect(schfn.__endpoint).to.deep.eq({
platform: "gcfv2",
labels: {},
scheduleTrigger: {
schedule: "* * * * *",
timeZone: undefined,
retryConfig: {
retryCount: undefined,
maxRetrySeconds: undefined,
minBackoffSeconds: undefined,
maxBackoffSeconds: undefined,
maxDoublings: undefined,
},
},
});
expect(schfn.__requiredAPIs).to.deep.eq([
{
api: "cloudscheduler.googleapis.com",
reason: "Needed for scheduled functions.",
},
]);
});

it("should have a .run method", async () => {
const testObj = {
foo: "bar",
Expand Down
67 changes: 67 additions & 0 deletions spec/v2/providers/tasks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,73 @@ describe("onTaskDispatched", () => {
});
});

it("should return a minimal endpoint without preserveExternalChanges set", () => {
const result = onTaskDispatched(
{
retryConfig: {
maxAttempts: 4,
maxRetrySeconds: 10,
},
rateLimits: {
maxDispatchesPerSecond: 10,
},
},
() => undefined
);

expect(result.__endpoint).to.deep.equal({
...MINIMAL_V2_ENDPOINT,
platform: "gcfv2",
labels: {},
taskQueueTrigger: {
retryConfig: {
maxAttempts: 4,
maxRetrySeconds: 10,
maxBackoffSeconds: options.RESET_VALUE,
maxDoublings: options.RESET_VALUE,
minBackoffSeconds: options.RESET_VALUE,
},
rateLimits: {
maxDispatchesPerSecond: 10,
maxConcurrentDispatches: options.RESET_VALUE,
},
},
});
});

it("should create a complex endpoint with preserveExternalChanges set", () => {
const result = onTaskDispatched(
{
...FULL_OPTIONS,
retryConfig: {
maxAttempts: 4,
maxRetrySeconds: 10,
},
rateLimits: {
maxDispatchesPerSecond: 10,
},
invoker: "private",
preserveExternalChanges: true,
},
() => undefined
);

expect(result.__endpoint).to.deep.equal({
...FULL_ENDPOINT,
platform: "gcfv2",
taskQueueTrigger: {
retryConfig: {
maxAttempts: 4,
maxRetrySeconds: 10,
},
rateLimits: {
maxDispatchesPerSecond: 10,
},
invoker: ["private"],
},
});
});

it("should merge options and globalOptions", () => {
options.setGlobalOptions({
concurrency: 20,
Expand Down
3 changes: 3 additions & 0 deletions spec/v2/providers/testLab.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import { expect } from "chai";
import * as testLab from "../../../src/v2/providers/testLab";
import * as options from "../../../src/v2/options";
import { MINIMAL_V2_ENDPOINT } from "../../fixtures";

describe("onTestMatrixCompleted", () => {
afterEach(() => {
Expand All @@ -33,6 +34,7 @@ describe("onTestMatrixCompleted", () => {
const fn = testLab.onTestMatrixCompleted(() => 2);

expect(fn.__endpoint).to.deep.eq({
...MINIMAL_V2_ENDPOINT,
platform: "gcfv2",
labels: {},
eventTrigger: {
Expand All @@ -59,6 +61,7 @@ describe("onTestMatrixCompleted", () => {
);

expect(fn.__endpoint).to.deep.eq({
...MINIMAL_V2_ENDPOINT,
platform: "gcfv2",
availableMemoryMb: 512,
region: ["us-central1"],
Expand Down
Loading