Skip to content
This repository was archived by the owner on Mar 4, 2026. It is now read-only.

fix: create new batch for writes to the same doc#1352

Merged
thebrianchen merged 2 commits intomasterfrom
bc/bulk-batching
Nov 3, 2020
Merged

fix: create new batch for writes to the same doc#1352
thebrianchen merged 2 commits intomasterfrom
bc/bulk-batching

Conversation

@thebrianchen
Copy link
Copy Markdown

@thebrianchen thebrianchen commented Nov 3, 2020

Fixing BulkWriter to add new writes to the same document into a new batch, since the BE endpoint validates that there are no duplicate docs in each batch.

(BulkCommitBatch refactoring coming next)

@thebrianchen thebrianchen requested review from a team November 3, 2020 19:21
@thebrianchen thebrianchen self-assigned this Nov 3, 2020
@product-auto-label product-auto-label Bot added the api: firestore Issues related to the googleapis/nodejs-firestore API. label Nov 3, 2020
@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 3, 2020
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 3, 2020

Codecov Report

Merging #1352 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1352   +/-   ##
=======================================
  Coverage   98.55%   98.55%           
=======================================
  Files          32       32           
  Lines       19274    19290   +16     
  Branches     1386     1387    +1     
=======================================
+ Hits        18995    19011   +16     
  Misses        276      276           
  Partials        3        3           
Impacted Files Coverage Δ
dev/src/bulk-writer.ts 99.80% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6fb360c...c1faf8a. Read the comment docs.

Comment thread dev/system-test/firestore.ts Outdated
expect(deleteResult.writeTime).to.deep.equal(new Timestamp(0, 0));
});

it('has can write to the same document twice', async () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, my amazing Meme doesn't seem to show up, but please drop "has".

@thebrianchen thebrianchen merged commit bd5adc3 into master Nov 3, 2020
@thebrianchen thebrianchen deleted the bc/bulk-batching branch November 3, 2020 20:18
gcf-owl-bot Bot added a commit that referenced this pull request Apr 7, 2022
Co-authored-by: Benjamin E. Coe <[email protected]>
Co-authored-by: Jeffrey Rennie <[email protected]>
Source-Link: googleapis/synthtool@e1557e4
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:c2641f0628c1077d504eca6b065517b0cc54336f09a9a38e3614c4b412dd9d37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: firestore Issues related to the googleapis/nodejs-firestore API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants