feat(functions): Add FunctionScope and update task_queue to support kit scopes#967
Open
inlined wants to merge 7 commits into
Open
feat(functions): Add FunctionScope and update task_queue to support kit scopes#967inlined wants to merge 7 commits into
inlined wants to merge 7 commits into
Conversation
…it scopes Added FunctionScope class, updated task_queue signature, and refactored TaskQueue to implement rollback-safe recursive fallback targeting kit queue when hitting a 404 response.
Factored out payload and HTTP request preparation in TaskQueue.enqueue and TaskQueue.delete into private helper methods _enqueue_with_scope and _delete_with_scope. This avoids recursion, class state pollution, and rollback logic, keeping self._scope unmodified until the request successfully completes.
Updated delete to raise NotFoundError on 404 response rather than silently swallowing it, preserving original SDK behavior. Added and updated tests to assert 404 propagation and rollback.
… and ensure complete thread safety without locks Imported Tuple from typing and updated _resolve_resource signature to support Python versions older than 3.9. Restored the accidentally removed handle_functions_error method body. Removed self._scope mutation entirely to ensure complete thread safety without locks, using a thread-safe global set _WARNED_INSTANCES to ensure each warning is only logged once.
…NCE_ID Renamed the KIT_INSTANCE_ID environment variable to FIREBASE_KIT_INSTANCE_ID in both functions.py and tests.
…fallback warning suggestion Updated _resolve_resource to support resolving current scope using EXT_INSTANCE_ID (checking it first before falling back to FIREBASE_KIT_INSTANCE_ID) for completeness and consistency. Updated _log_fallback_warning message to suggest the more idiomatic functions.FunctionScope.
There was a problem hiding this comment.
Code Review
This pull request introduces the FunctionScope class to define function scopes in task queues, replacing the deprecated extension_id parameter in task_queue with a new scope parameter. It also implements fallback logic to retry with 'kit' scope upon receiving a 404 error for 'extension_or_kit' scopes, and adds comprehensive unit tests. The feedback suggests refactoring the error handling in the delete method to use a guard-clause style for consistency with the enqueue method.
…e style Refactored the delete method in TaskQueue to use a guard-clause style for handling exceptions, aligning it with enqueue and removing the redundant return statement.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
This PR introduces support for explicit task queue scopes to match the design patterns introduced in
firebase-admin-node#3210.It adds the new
FunctionScopeclass and updates thetask_queuesignature to accept ascopeparameter (while deprecatingextension_id). It also implements an automatic fallback retry flow that shifts queue targets from extensions to kits upon receiving a404 Not Foundresponse.To ensure complete thread safety without requiring locks, the
TaskQueueclass is stateless/immutable, delegating requests to private helper functions_enqueue_with_scopeand_delete_with_scopethat accept the resolved scope as a parameter.Key Changes:
FunctionScope: Introduced a validation-enforced scope type representing'current','global','extension', and'kit'.task_queueupdate: Added thescopeparameter, deprecatedextension_id, and bound warnings appropriately.enqueueanddeletemethods to use private helper functions. If a hybrid scope ('extension_or_kit') hits a404, it retries targeting the kit scope. If the fallback kit queue deletes fail with404, theNotFoundErrorexception is correctly propagated back to the user (preserving original Python SDK behavior).KIT_INSTANCE_IDtoFIREBASE_KIT_INSTANCE_IDand added support forEXT_INSTANCE_IDincurrentscope (with extension taking precedence).Tupleimport fromtypingand updated signatures to support Python versions older than 3.9 (e.g. 3.7 and 3.8).Testing Strategy
TestFunctionScope) covering constructor validations and bounds.FIREBASE_KIT_INSTANCE_ID,EXT_INSTANCE_IDand precedence).MockMultiRequestAdapterverifying that404errors triggers fallbacks, that successful retries log user warnings, and that failures propagate properly.10.00/10.