fix(firestore-incremental-capture): stop double-prefixing task queue names - #3117
fix(firestore-incremental-capture): stop double-prefixing task queue names#3117IzaakGough wants to merge 2 commits into
Conversation
…names queueName() prepended kit-<instance id>- to the queue, and the Admin SDK prepends it again from FIREBASE_KIT_INSTANCE_ID, so every enqueue targeted kit-<id>-kit-<id>-<function> and failed with "Queue does not exist". The new test drives the real Admin SDK against a local Cloud Tasks emulator host and asserts the URL it produces; the existing unit tests only checked the string queueName() returns, which is why this got through.
There was a problem hiding this comment.
Code Review
This pull request fixes a task dispatch failure ("Queue does not exist") caused by double-prefixing queue names. The manual kit-<instance id>- prefix has been removed from the queueName builder, allowing the Admin SDK to handle the prefixing automatically via FIREBASE_KIT_INSTANCE_ID. The changes also include updates to documentation, JSDoc comments, existing tests, and the addition of a new test suite to verify correct queue targeting. I have no feedback to provide as there are no review comments.
…t credential Under the default credential the Admin SDK reaches for application default credentials while building the task payload, so the test only passed on a machine that had run `gcloud auth`. The emulator path never uses the credential; it sends an "owner" token.
|
This fix requires Version 14.1.x does not read Please bump the minimum to |
Fixes #3113.
What was broken
queueName()prependedkit-<instance id>-to the queue name, and the Admin SDK prepends the same prefix fromFIREBASE_KIT_INSTANCE_IDinresolveResourceId(). Every enqueue targetedkit-<id>-kit-<id>-<function>and failed with "Queue does not exist", so changelog rows never reached BigQuery and restorations never started.What changed
queueName()returns the bare export name and lets the SDK add the prefix.INSTANCE_IDstill namespaces the Dataflow jobs, the flex template object and the run-status documents, so it stays required; the doc comments and README that described it as naming the task queues are updated.How it was verified
tests/tasks.enqueue.test.tspointsCLOUD_TASKS_EMULATOR_HOSTat a local server and drives the real Admin SDK, asserting the queue URL it produces. It fails on the old code with the doubled prefix and passes on the new one. It also pins the case the fix cannot distinguish: withFIREBASE_KIT_INSTANCE_IDunset there is no prefix at all, so the kit has no fallback of its own.The existing unit tests only checked the string
queueName()returned, which is why this got through. Not exercised here: a live deploy. The reporter confirmed the same change works against one.