feat(functions): Add FunctionScope and update task_queue to support kit scopes#966
feat(functions): Add FunctionScope and update task_queue to support kit scopes#966inlined wants to merge 6 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.
![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 the FunctionScope class to manage task queue scopes ('current', 'global', 'extension', and 'kit') and deprecates the extension_id parameter in favor of scope. Feedback highlights critical compatibility issues with Python versions older than 3.9 due to the use of tuple[...] type hinting, a bug in the delete method's error handling that silently swallows 404 errors and breaks scope rollback logic, and thread-safety concerns from mutating self._scope concurrently. Additionally, a test case asserting incorrect 404-swallowing behavior needs to be updated.
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.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the FunctionScope class to define the scope of a function in a task queue, deprecating the extension_id parameter in task_queue in favor of a more flexible scope parameter. It also implements a fallback mechanism to retry with kit scope on 404 errors and adds corresponding unit tests. The review feedback suggests supporting EXT_INSTANCE_ID in the current scope for consistency with other SDKs, and updating the deprecation warning message to recommend functions.FunctionScope to avoid potential NameErrors.
…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.
1 participant
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, rollback-safe fallback retry flow that shifts queue targets from extensions to kits upon receiving a404 Not Foundresponse.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 attempt a recursive kit-retry if a hybrid scope hits a404. If the retry fails, the scope change is safely rolled back to prevent state contamination, and the fallback warning is only logged on successful execution.Testing Strategy
TestFunctionScope) covering constructor validations and bounds.KIT_INSTANCE_ID).MockMultiRequestAdapterverifying that404errors triggers fallbacks, that successful retries log user warnings, and that failures rollback scope changes.10.00/10.