- Notifications
You must be signed in to change notification settings - Fork 217
Insert Expression and Field types into GlobalOptions and inheriting types #1171
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
Changes from 1 commit
3ad62e2 5f8cccb b7f879b 48f2d9a bf6c420 b6d592e e274fc7 c00c2f7 66cffc9 156a21d 1089043 b0a9063 8942e5c 3778deb 6379cc3 27937eb df07167 aaf3dda d4bf770 a114386 8c3447b f63c895 2c67693 251aa43 47e2330 File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| /* | ||
| * A CEL expression which can be evaulated during function deployment, and | ||
Berlioz marked this conversation as resolved. Outdated Show resolved Hide resolved | ||
| * resolved to a value of the generic type parameter: i.e, you can pass | ||
| * an Expression<number> as the value of an option that normally accepts numbers. | ||
| ||
| */ | ||
| export type Expression<T extends string | number | boolean> = string; // eslint-disable-line | ||
| ||
| | ||
| export type Field<T extends string | number | boolean> = T | Expression<T> | null; | ||
| | ||
| /** | ||
| * Casts an Expression to its literal string value, for use by __getSpec() | ||
| * @internal | ||
| */ | ||
| export function ExprString<Expression>(arg: Expression): string { | ||
| return arg as unknown as string; | ||
| } | ||
| | ||
| /** | ||
| * Sanity check on whether a CEL expression is actually evaluatable | ||
| * @internal | ||
| */ | ||
| export function IsValid<Expression>(arg: Expression): boolean { | ||
| return true; | ||
Berlioz marked this conversation as resolved. Outdated Show resolved Hide resolved | ||
| } | ||
Berlioz marked this conversation as resolved. Outdated Show resolved Hide resolved | ||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| | @@ -37,6 +37,7 @@ import { TriggerAnnotation } from './core'; | |||
| import { declaredParams } from './params'; | ||||
| import { ParamSpec } from './params/types'; | ||||
| import { HttpsOptions } from './providers/https'; | ||||
| import { Expression, Field, ExprString } from './expressions'; | ||||
| | ||||
| /** | ||||
| * List of all regions supported by Cloud Functions v2 | ||||
| | @@ -76,6 +77,10 @@ const MemoryOptionToMB: Record<MemoryOption, number> = { | |||
| '32GiB': 32768, | ||||
| }; | ||||
| | ||||
| function isMemoryOption(arg: MemoryOption | Expression<number>): arg is MemoryOption { | ||||
| return (arg in MemoryOptionToMB); | ||||
| } | ||||
| | ||||
| /** | ||||
| * List of available options for VpcConnectorEgressSettings. | ||||
| */ | ||||
| | @@ -104,7 +109,7 @@ export interface GlobalOptions { | |||
| * Amount of memory to allocate to a function. | ||||
| * A value of null restores the defaults of 256MB. | ||||
| */ | ||||
| memory?: MemoryOption | null; | ||||
| memory?: MemoryOption | Expression<number> | null; | ||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aren't memoryOptions string? firebase-functions/src/v2/options.ts Line 56 in 27a4983
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see. I think this is a bad footgun. For example, I would write something like this: And this totally wouldn't work no? It would have to be instead: or something which feels surprising. | ||||
| | ||||
| /** | ||||
| * Timeout for the function in sections, possible values are 0 to 540. | ||||
| | @@ -116,21 +121,21 @@ export interface GlobalOptions { | |||
| * maximum timeout of 36,00s (1 hour). Task queue functions have a maximum | ||||
| * timeout of 1,800s (30 minutes) | ||||
| */ | ||||
| timeoutSeconds?: number | null; | ||||
| timeoutSeconds?: Field<number>; | ||||
| | ||||
| /** | ||||
| * Min number of actual instances to be running at a given time. | ||||
| * Instances will be billed for memory allocation and 10% of CPU allocation | ||||
| * while idle. | ||||
| * A value of null restores the default min instances. | ||||
| */ | ||||
| minInstances?: number | null; | ||||
| minInstances?: Field<number>; | ||||
| | ||||
| /** | ||||
| * Max number of instances to be running in parallel. | ||||
| * A value of null restores the default max instances. | ||||
| */ | ||||
| maxInstances?: number | null; | ||||
| maxInstances?: Field<number>; | ||||
| | ||||
| /** | ||||
| * Number of requests a function can serve at once. | ||||
| | @@ -139,7 +144,7 @@ export interface GlobalOptions { | |||
| * Concurrency cannot be set to any value other than 1 if `cpu` is less than 1. | ||||
| * The maximum value for concurrency is 1,000. | ||||
| */ | ||||
| concurrency?: number | null; | ||||
| concurrency?: Field<number>; | ||||
| | ||||
| /** | ||||
| * Fractional number of CPUs to allocate to a function. | ||||
| | @@ -246,8 +251,8 @@ export function optionsToTriggerAnnotations( | |||
| opts, | ||||
| 'availableMemoryMb', | ||||
| 'memory', | ||||
| (mem: MemoryOption) => { | ||||
| return MemoryOptionToMB[mem]; | ||||
| (mem: MemoryOption | Expression<number>): number | string => { | ||||
| return isMemoryOption(mem) ? MemoryOptionToMB[mem] : ExprString(mem); | ||||
Berlioz marked this conversation as resolved. Outdated Show resolved Hide resolved | ||||
| } | ||||
| ); | ||||
| convertIfPresent(annotation, opts, 'regions', 'region', (region) => { | ||||
| | @@ -308,8 +313,8 @@ export function optionsToEndpoint( | |||
| convertIfPresent(vpc, opts, 'egressSettings', 'vpcConnectorEgressSettings'); | ||||
| endpoint.vpc = vpc; | ||||
| } | ||||
| convertIfPresent(endpoint, opts, 'availableMemoryMb', 'memory', (mem) => { | ||||
| return MemoryOptionToMB[mem]; | ||||
| convertIfPresent(endpoint, opts, 'availableMemoryMb', 'memory', (mem: MemoryOption | Expression<number>): number | string => { | ||||
| return isMemoryOption(mem) ? MemoryOptionToMB[mem] : ExprString(mem); | ||||
| }); | ||||
| convertIfPresent(endpoint, opts, 'region', 'region', (region) => { | ||||
| if (typeof region === 'string') { | ||||
| | ||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't use
TriggerAnnotationanymore, so you can revert these changes