feat(functions): Implement TaskQueue Scopes#3210
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new FunctionScope type to represent the scope of a function in a task queue, deprecating the legacy extensionId string parameter. It updates FunctionsApiClient, Functions, and TaskQueue to support these scopes (including current, global, extension, and internal kit scopes) with fallback logic for migration, and adds comprehensive unit tests. The review feedback highlights a few issues and improvement opportunities: correcting a typo ('peformance') and an invalid scope value ({ scope: "self" } instead of { scope: "current" }) in warning messages (and their corresponding tests), removing a redundant type check in TaskQueue, and improving type safety in FunctionsApiClient by avoiding an as any cast through discriminated union narrowing.
lahirumaramba
left a comment
lahirumaramba
left a comment
There was a problem hiding this comment.
Thank you for submitting this!
LGTM! Added a couple of comments.
| functionName: string, | ||
| extensionIdOrScope: string | FunctionScope = { scope: 'current' } | ||
| ): TaskQueue<Args> { | ||
| const scope: FunctionScope = typeof extensionIdOrScope === 'string' |
There was a problem hiding this comment.
If we use FunctionScope will that get rid of the as any casts?
| scope?: InternalFunctionScope | ||
| ): { resourceId: string; extensionOrKitId?: string } { | ||
| if (typeof scope === 'undefined') { | ||
| return this.resolveResourceId(resourceId, { scope: 'current' }); |
There was a problem hiding this comment.
Should we use a default parameter for scope and skip this recursive call?
private resolveResourceId (
resourceId : string ,
scope : InternalFunctionScope = { scope: 'current' }
)...| * @param id - The ID of the task to delete. | ||
| * @param functionName - The function name of the queue. | ||
| * @param extensionId - Optional canonical ID of the extension. | ||
| * @param scope - Optional FunctionScope configuration. |
There was a problem hiding this comment.
Could we get a TW review on these new docstrings, please?
2 participants
Description
This PR introduces Task Queue Scopes to the Firebase Admin SDK functions module. It decouples the legacy backward-compatible string-to-scope migration logic from the stateless client and isolates it inside the API wrapper layer (
TaskQueue).Key Changes
TaskQueue):TaskQueue.enqueue()anddelete()now capture404errors for legacy string parameters ('extensionOrKit'scope).404error, they automatically retry using a temporarykitscope.'kit'and logs a legacy targeting warning.process.env.EXT_INSTANCE_IDandprocess.env.KIT_INSTANCE_IDat construction time.'extension'or'kit', bypassing unnecessary404fallback call roundtrips.scoperesolves to global if no environment variables are set.