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

Add output bundle spec to common module #244

Merged
merged 14 commits into from
Sep 3, 2024
53 changes: 53 additions & 0 deletions packages/@apphosting/common/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,58 @@
import { spawn } from "child_process";

// Output bundle specifications to be written to bundle.yaml
export interface OutputBundle {
tonyjhuang marked this conversation as resolved.
Show resolved Hide resolved
version: "v1alpha";
tonyjhuang marked this conversation as resolved.
Show resolved Hide resolved
serverConfig: ServerConfig;
metadata: Metadata;
}

// Fields needed to configure the App Hosting server
interface ServerConfig {
// Command to start the server (e.g. ["node", "dist/index.js"]) assume this command is run from the root dir of the workspace
tonyjhuang marked this conversation as resolved.
Show resolved Hide resolved
runCommand: string[];
tonyjhuang marked this conversation as resolved.
Show resolved Hide resolved
// Environment variable present in the server execution environment.
tonyjhuang marked this conversation as resolved.
Show resolved Hide resolved
environmentVariables?: EnvVarConfig[];
// The maximum number of concurrent requests that each server instance can receive.
// Defaults to 80.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to drop these defaults now that we know GCP can adjust default values per user?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The defaults are set the same way the apphosting.yaml defaults are set. The comments are more just a fyi. Do they no longer apply? If not we should probably change the public docs too to reflect that https://firebase.google.com/docs/app-hosting/configure#configure-backend

Copy link
Collaborator

Choose a reason for hiding this comment

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

What if we link to the product docs instead so we have a single source of truth?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed the defaults in favor of linking the docs.

concurrency?: number;
// The number of CPUs used in a single server instance.
tonyjhuang marked this conversation as resolved.
Show resolved Hide resolved
// Defaults to 1.
cpu?: "fractional" | 1 | 2 | 4 | 6 | 8;
// The amount of memory available for a server instance.
// Commonly one of: "256" | "512" | "1024" | "2048" | "4096" | "8192" | "16384"
// Defaults to 512MiB.
memoryMiB?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, we probably want number here too

// The limit on the minimum number of function instances that may coexist at a given time.
// Defaults to 0.
minInstances?: number;
// The limit on the maximum number of function instances that may coexist at a given time.
// Defaults to 100.
maxInstances?: number;
}

// Additonal fields needed for adapter identification
tonyjhuang marked this conversation as resolved.
Show resolved Hide resolved
interface Metadata {
// Name of the adapter (this should be the npm package name)
adapterNpmPackageName: string;
// Version of the adapter
tonyjhuang marked this conversation as resolved.
Show resolved Hide resolved
adapterVersion?: string;
tonyjhuang marked this conversation as resolved.
Show resolved Hide resolved
// Name of the framework that is being supported
framework: string;
// Version of the framework that is being supported
frameworkVersion?: string;
sjjj986 marked this conversation as resolved.
Show resolved Hide resolved
}

// Configs necessary to define environment variables
tonyjhuang marked this conversation as resolved.
Show resolved Hide resolved
interface EnvVarConfig {
// Name of the variable
variable: string;
// Value associated with the variable
value: string;
// Where the variable will be available, for now this will always be RUNTIME
availability: "RUNTIME"[];
tonyjhuang marked this conversation as resolved.
Show resolved Hide resolved
}

// Options to configure the build of a framework application
export interface BuildOptions {
// command to run build script (e.g. "npm", "nx", etc.)
Expand Down
Loading