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

Implement Experiments feature #4994

Merged
merged 19 commits into from Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 0 additions & 2 deletions .github/workflows/node-test.yml
Expand Up @@ -61,7 +61,6 @@ jobs:
runs-on: ubuntu-latest

env:
FIREBASE_CLI_PREVIEWS: none
FIREBASE_EMULATORS_PATH: ${{ github.workspace }}/emulator-cache
COMMIT_SHA: ${{ github.sha }}
CI_JOB_ID: ${{ github.action }}
Expand Down Expand Up @@ -116,7 +115,6 @@ jobs:
runs-on: windows-latest

env:
FIREBASE_CLI_PREVIEWS: none
FIREBASE_EMULATORS_PATH: ${{ github.workspace }}/emulator-cache
COMMIT_SHA: ${{ github.sha }}
CI_JOB_ID: ${{ github.action }}
Expand Down
6 changes: 3 additions & 3 deletions src/commands/database-instances-list.ts
Expand Up @@ -9,7 +9,7 @@ import { needProjectNumber } from "../projectUtils";
import firedata = require("../gcp/firedata");
import { Emulators } from "../emulator/types";
import { warnEmulatorNotSupported } from "../emulator/commandUtils";
import { previews } from "../previews";
import * as experiments from "../experiments";
import { needProjectId } from "../projectUtils";
import {
listDatabaseInstances,
Expand Down Expand Up @@ -52,7 +52,7 @@ export let command = new Command("database:instances:list")
).start();
let instances;

if (previews.rtdbmanagement) {
if (experiments.isEnabled("rtdbmanagement")) {
const projectId = needProjectId(options);
try {
instances = await listDatabaseInstances(projectId, location);
Expand Down Expand Up @@ -80,7 +80,7 @@ export let command = new Command("database:instances:list")
return instances;
});

if (previews.rtdbmanagement) {
if (experiments.isEnabled("rtdbmanagement")) {
command = command.option(
"-l, --location <location>",
"(optional) location for the database instance, defaults to us-central1"
Expand Down
4 changes: 2 additions & 2 deletions src/commands/ext-install.ts
Expand Up @@ -30,7 +30,7 @@ import { getRandomString } from "../extensions/utils";
import { requirePermissions } from "../requirePermissions";
import * as utils from "../utils";
import { track } from "../track";
import { previews } from "../previews";
import * as experiments from "../experiments";
import { Options } from "../options";
import * as manifest from "../extensions/manifest";

Expand All @@ -44,7 +44,7 @@ marked.setOptions({
export const command = new Command("ext:install [extensionName]")
.description(
"install an official extension if [extensionName] or [extensionName@version] is provided; " +
(previews.extdev
(experiments.isEnabled("extdev")
? "install a local extension if [localPathOrUrl] or [url#root] is provided; install a published extension (not authored by Firebase) if [publisherId/extensionId] is provided "
: "") +
"or run with `-i` to see all available extensions."
Expand Down
4 changes: 2 additions & 2 deletions src/commands/ext-update.ts
Expand Up @@ -22,7 +22,7 @@ import * as refs from "../extensions/refs";
import { getProjectId } from "../projectUtils";
import { requirePermissions } from "../requirePermissions";
import * as utils from "../utils";
import { previews } from "../previews";
import * as experiments from "../experiments";
import * as manifest from "../extensions/manifest";
import { Options } from "../options";
import * as askUserForEventsConfig from "../extensions/askUserForEventsConfig";
Expand All @@ -36,7 +36,7 @@ marked.setOptions({
*/
export const command = new Command("ext:update <extensionInstanceId> [updateSource]")
.description(
previews.extdev
experiments.isEnabled("extdev")
? "update an existing extension instance to the latest version or from a local or URL source"
: "update an existing extension instance to the latest version"
)
Expand Down
13 changes: 8 additions & 5 deletions src/commands/index.ts
@@ -1,5 +1,8 @@
import { previews } from "../previews";
import * as experiments from "../experiments";

/**
* Loads all commands for our parser.
*/
export function load(client: any): any {
function loadCommand(name: string) {
const t0 = process.hrtime.bigint();
Expand Down Expand Up @@ -48,7 +51,7 @@ export function load(client: any): any {
client.database.profile = loadCommand("database-profile");
client.database.push = loadCommand("database-push");
client.database.remove = loadCommand("database-remove");
if (previews.rtdbrules) {
if (experiments.isEnabled("rtdbrules")) {
client.database.rules = {};
client.database.rules.get = loadCommand("database-rules-get");
client.database.rules.list = loadCommand("database-rules-list");
Expand Down Expand Up @@ -77,11 +80,11 @@ export function load(client: any): any {
client.ext.list = loadCommand("ext-list");
client.ext.uninstall = loadCommand("ext-uninstall");
client.ext.update = loadCommand("ext-update");
if (previews.ext) {
if (experiments.isEnabled("ext")) {
client.ext.sources = {};
client.ext.sources.create = loadCommand("ext-sources-create");
}
if (previews.extdev) {
if (experiments.isEnabled("extdev")) {
client.ext.dev = {};
client.ext.dev.init = loadCommand("ext-dev-init");
client.ext.dev.list = loadCommand("ext-dev-list");
Expand Down Expand Up @@ -110,7 +113,7 @@ export function load(client: any): any {
client.functions.log = loadCommand("functions-log");
client.functions.shell = loadCommand("functions-shell");
client.functions.list = loadCommand("functions-list");
if (previews.deletegcfartifacts) {
if (experiments.isEnabled("deletegcfartifacts")) {
client.functions.deletegcfartifacts = loadCommand("functions-deletegcfartifacts");
}
client.functions.secrets = {};
Expand Down
4 changes: 2 additions & 2 deletions src/deploy/functions/build.ts
Expand Up @@ -2,7 +2,7 @@ import * as backend from "./backend";
import * as proto from "../../gcp/proto";
import * as api from "../../.../../api";
import * as params from "./params";
import { previews } from "../../previews";
import * as experiments from "../../experiments";
import { FirebaseError } from "../../error";
import { assertExhaustive, mapObject, nullsafeVisitor } from "../../functional";
import { UserEnvsOpts, writeUserEnvs } from "../../functions/env";
Expand Down Expand Up @@ -281,7 +281,7 @@ export async function resolveBackend(
nonInteractive?: boolean
): Promise<{ backend: backend.Backend; envs: Record<string, params.ParamValue> }> {
let paramValues: Record<string, params.ParamValue> = {};
if (previews.functionsparams) {
if (experiments.isEnabled("functionsparams")) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Berlioz Does this still need to be hidden behind an experiment or do we think we are stable enough to roll forward? (If not, can we leave a comment on when this should be rolled forward?)

paramValues = await params.resolveParams(
build.params,
firebaseConfig,
Expand Down
7 changes: 5 additions & 2 deletions src/deploy/functions/prepare.ts
Expand Up @@ -29,7 +29,7 @@ import { FirebaseError } from "../../error";
import { configForCodebase, normalizeAndValidate } from "../../functions/projectConfig";
import { AUTH_BLOCKING_EVENTS } from "../../functions/events/v1";
import { generateServiceIdentity } from "../../gcp/serviceusage";
import { previews } from "../../previews";
import * as experiments from "../../experiments";
import { applyBackendHashToBackends } from "./cache/applyHash";
import { allEndpoints, Backend } from "./backend";

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheIronDev When does this experiment go away?

applyBackendHashToBackends(wantBackends, context);
}
}
Expand Down Expand Up @@ -346,6 +346,9 @@ function maybeCopyTriggerRegion(wantE: backend.Endpoint, haveE: backend.Endpoint
wantE.eventTrigger.region = haveE.eventTrigger.region;
}

/**
* Determines whether endpoints are targeted by an --only flag.
*/
export function updateEndpointTargetedStatus(
wantBackends: Record<string, Backend>,
endpointFilters: EndpointFilter[]
Expand Down
14 changes: 12 additions & 2 deletions src/deploy/functions/release/planner.ts
@@ -1,3 +1,5 @@
import * as clc from "colorette";

import {
EndpointFilter,
endpointMatchesAnyFilter,
Expand All @@ -8,7 +10,7 @@ import { FirebaseError } from "../../../error";
import * as utils from "../../../utils";
import * as backend from "../backend";
import * as v2events from "../../../functions/events/v2";
import { previews } from "../../../previews";
import * as experiments from "../../../experiments";

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

const { skipdeployingnoopfunctions } = previews;
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 =>
Expand All @@ -75,6 +77,14 @@ export function calculateChangesets(
}, {});

const toSkip = utils.groupBy(Object.values(toSkipEndpointsMap), keyFn);
if (Object.keys(toSkip).length) {
utils.logLabeledBullet(
"functions",
`Skipping the deploy of unchanged functions with ${clc.bold(
"experimental"
)} support for skipdeployingnoopfunctions`
);
}

const toUpdate = utils.groupBy(
Object.keys(want)
Expand Down
5 changes: 3 additions & 2 deletions src/deploy/index.ts
Expand Up @@ -7,7 +7,7 @@ import { logBullet, logSuccess, consoleUrl, addSubdomain } from "../utils";
import { FirebaseError } from "../error";
import { track } from "../track";
import { lifecycleHooks } from "./lifecycleHooks";
import { previews } from "../previews";
import * as experiments from "../experiments";
import * as HostingTarget from "./hosting";
import * as DatabaseTarget from "./database";
import * as FirestoreTarget from "./firestore";
Expand Down Expand Up @@ -56,9 +56,10 @@ export const deploy = async function (
const postdeploys: Chain = [];
const startTime = Date.now();

if (previews.frameworkawareness && targetNames.includes("hosting")) {
if (targetNames.includes("hosting")) {
inlined marked this conversation as resolved.
Show resolved Hide resolved
const config = options.config.get("hosting");
if (Array.isArray(config) ? config.some((it) => it.source) : config.source) {
experiments.assertEnabled("frameworkawareness", "deploy a web framework to hosting");
await prepareFrameworks(targetNames, context, options);
}
}
Expand Down
14 changes: 8 additions & 6 deletions src/emulator/controller.ts
Expand Up @@ -51,7 +51,7 @@ import { ExtensionsEmulator } from "./extensionsEmulator";
import { normalizeAndValidate } from "../functions/projectConfig";
import { requiresJava } from "./downloadableEmulators";
import { prepareFrameworks } from "../frameworks";
import { previews } from "../previews";
import * as experiments from "../experiments";

const START_LOGGING_EMULATOR = utils.envOverride(
"START_LOGGING_EMULATOR",
Expand Down Expand Up @@ -323,6 +323,9 @@ interface EmulatorOptions extends Options {
extDevEnv?: Record<string, string>;
}

/**
* Start all emulators.
*/
export async function startAll(
options: EmulatorOptions,
showUI = true
Expand Down Expand Up @@ -395,11 +398,10 @@ export async function startAll(
}
}

if (previews.frameworkawareness) {
const config = options.config.get("hosting");
if (Array.isArray(config) ? config.some((it) => it.source) : config?.source) {
await prepareFrameworks(targets, options, options);
}
const config = options.config.get("hosting");
if (Array.isArray(config) ? config.some((it) => it.source) : config?.source) {
experiments.assertEnabled("frameworkawareness", "emulate a web framework with hosting");
await prepareFrameworks(targets, options, options);
}

function startEmulator(instance: EmulatorInstance): Promise<void> {
Expand Down
10 changes: 8 additions & 2 deletions src/emulator/downloadableEmulators.ts
Expand Up @@ -18,7 +18,7 @@ import * as path from "path";
import * as os from "os";
import { EmulatorRegistry } from "./registry";
import { downloadEmulator } from "../emulator/download";
import { previews } from "../previews";
import * as experiments from "../experiments";

const EMULATOR_INSTANCE_KILL_TIMEOUT = 4000; /* ms */

Expand Down Expand Up @@ -62,7 +62,7 @@ export const DownloadDetails: { [s in DownloadableEmulators]: EmulatorDownloadDe
namePrefix: "cloud-storage-rules-emulator",
},
},
ui: previews.emulatoruisnapshot
ui: experiments.isEnabled("emulatoruisnapshot")
? {
version: "SNAPSHOT",
downloadPath: path.join(CACHE_DIR, "ui-vSNAPSHOT.zip"),
Expand Down Expand Up @@ -286,6 +286,9 @@ async function _fatal(emulator: Emulators, errorMsg: string): Promise<void> {
}
}

/**
* Handle errors in an emulator process.
*/
export async function handleEmulatorProcessError(emulator: Emulators, err: any): Promise<void> {
const description = Constants.description(emulator);
if (err.path === "java" && err.code === "ENOENT") {
Expand All @@ -298,6 +301,9 @@ export async function handleEmulatorProcessError(emulator: Emulators, err: any):
}
}

/**
* Do the selected list of emulators depend on the JRE.
*/
export function requiresJava(emulator: Emulators): boolean {
if (emulator in Commands) {
return Commands[emulator as keyof typeof Commands].binary === "java";
Expand Down
9 changes: 5 additions & 4 deletions src/emulator/storage/rules/runtime.ts
Expand Up @@ -32,10 +32,10 @@ import {
} from "../../downloadableEmulators";
import { EmulatorRegistry } from "../../registry";
import { Client } from "../../../apiv2";
import { previews } from "../../../previews";
import * as experiments from "../../../experiments";

const lock = new AsyncLock();
const synchonizationKey: string = "key";
const synchonizationKey = "key";

export interface RulesetVerificationOpts {
file: {
Expand Down Expand Up @@ -443,8 +443,9 @@ async function fetchFirestoreDocument(
projectId: string,
request: RuntimeActionFirestoreDataRequest
): Promise<RuntimeActionFirestoreDataResponse> {
// If preview not enabled, just throw an error
if (!previews.crossservicerules) {
// If experiment is not enabled, just throw an error
// TODO: should this be an assertEnabled instead?
if (!experiments.isEnabled("crossservicerules")) {
return { status: DataLoadStatus.INVALID_STATE, warnings: [], errors: [] };
}

Expand Down