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

[Function-NoOp] Enable Skipping NoOp Deploys by Default #5032

Merged
merged 2 commits into from Sep 29, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
@@ -1 +1,2 @@
- Add the "experiments" family of commands (#4994)
- Enable detecting and skipping no-op function deploys (#5032).
5 changes: 1 addition & 4 deletions src/deploy/functions/prepare.ts
Expand Up @@ -29,7 +29,6 @@ import { FirebaseError } from "../../error";
import { configForCodebase, normalizeAndValidate } from "../../functions/projectConfig";
import { AUTH_BLOCKING_EVENTS } from "../../functions/events/v1";
import { generateServiceIdentity } from "../../gcp/serviceusage";
import * as experiments from "../../experiments";
import { applyBackendHashToBackends } from "./cache/applyHash";
import { allEndpoints, Backend } from "./backend";

Expand Down Expand Up @@ -272,9 +271,7 @@ export async function prepare(
* This must be called after `await validate.secretsAreValid`.
*/
updateEndpointTargetedStatus(wantBackends, context.filters || []);
if (experiments.isEnabled("skipdeployingnoopfunctions")) {
applyBackendHashToBackends(wantBackends, context);
}
applyBackendHashToBackends(wantBackends, context);
}

/**
Expand Down
4 changes: 0 additions & 4 deletions src/deploy/functions/release/planner.ts
Expand Up @@ -10,7 +10,6 @@ import { FirebaseError } from "../../../error";
import * as utils from "../../../utils";
import * as backend from "../backend";
import * as v2events from "../../../functions/events/v2";
import * as experiments from "../../../experiments";

export interface EndpointUpdate {
endpoint: backend.Endpoint;
Expand Down Expand Up @@ -56,12 +55,9 @@ export function calculateChangesets(
keyFn
);

const skipdeployingnoopfunctions = experiments.isEnabled("skipdeployingnoopfunctions");

// If the hashes are matching, that means the local function is the same as the server copy.
const toSkipPredicate = (id: string): boolean =>
!!(
skipdeployingnoopfunctions &&
!want[id].targetedByOnly && // Don't skip the function if its --only targeted.
have[id].hash &&
want[id].hash &&
Expand Down
32 changes: 0 additions & 32 deletions src/test/deploy/functions/release/planner.spec.ts
Expand Up @@ -6,7 +6,6 @@ import * as planner from "../../../../deploy/functions/release/planner";
import * as deploymentTool from "../../../../deploymentTool";
import * as utils from "../../../../utils";
import * as v2events from "../../../../functions/events/v2";
import * as experiments from "../../../../experiments";

describe("planner", () => {
let logLabeledBullet: sinon.SinonStub;
Expand All @@ -16,12 +15,10 @@ describe("planner", () => {
}

beforeEach(() => {
experiments.setEnabled("skipdeployingnoopfunctions", true);
logLabeledBullet = sinon.stub(utils, "logLabeledBullet");
});

afterEach(() => {
experiments.setEnabled("skipdeployingnoopfunctions", false);
sinon.verifyAndRestore();
});

Expand Down Expand Up @@ -225,33 +222,6 @@ describe("planner", () => {
});
});

it("does not add endpoints to skip list if preview flag is false", () => {
// Note: the two functions share the same id
const updatedWant = func("updated", "region");
const updatedHave = func("updated", "region");
// But their hash are the same (aka a no-op function)
updatedWant.hash = "to_skip";
updatedHave.hash = "to_skip";

experiments.setEnabled("skipdeployingnoopfunctions", false);

const want = { updated: updatedWant };
const have = { updated: updatedHave };

expect(planner.calculateChangesets(want, have, (e) => e.region)).to.deep.equal({
region: {
endpointsToCreate: [],
endpointsToUpdate: [
{
endpoint: updatedWant,
},
],
endpointsToDelete: [],
endpointsToSkip: [],
},
});
});

it("does not add endpoints to skip list if not targeted for deploy", () => {
// Note: the two functions share the same id
const updatedWant = func("updated", "region");
Expand All @@ -261,8 +231,6 @@ describe("planner", () => {
updatedHave.hash = "to_skip";
updatedWant.targetedByOnly = true;

experiments.setEnabled("skipdeployingnoopfunctions", true);

const want = { updated: updatedWant };
const have = { updated: updatedHave };

Expand Down