Changes the logic to use len(batch) to determine when we have reached write_batch_size instead of the iteration variable.
Adds assertions to how large each batch is that was written.
A few changes to comments.
Motivation
I was investigating the test cases and noted that the first batch written was always 1 more than the write_batch_size. In particular, I noticed that for TestChunkedWritesRespectContextCancellation the metricsFlushed was 4, despite the batch_size being 3. This seems incorrect / unintended behavior to me.
Test plan
Updated the test cases to assert on the batch size. Just using unit tests.
Summary
len(batch)
to determine when we have reachedwrite_batch_size
instead of the iteration variable.Motivation
I was investigating the test cases and noted that the first batch written was always 1 more than the
write_batch_size
. In particular, I noticed that forTestChunkedWritesRespectContextCancellation
the metricsFlushed was 4, despite the batch_size being 3. This seems incorrect / unintended behavior to me.Test plan
Updated the test cases to assert on the batch size. Just using unit tests.
Rollout/monitoring/revert plan