Skip to content

feat: Support BatchWrite API#99

Merged
dazuma merged 26 commits into
mainfrom
batch-write-api
Jun 25, 2024
Merged

feat: Support BatchWrite API#99
dazuma merged 26 commits into
mainfrom
batch-write-api

Conversation

@SandeepTuniki

@SandeepTuniki SandeepTuniki commented Apr 15, 2024

Copy link
Copy Markdown
Contributor

Implementation of the BatchWrite API. See internal link go/cs-batch-write-client-spec for details. This pull request includes the implementation and tests but not the samples (which will appear in https://github.com/GoogleCloudPlatform/ruby-docs-samples).

@dazuma dazuma marked this pull request as ready for review June 13, 2024 22:54
@dazuma dazuma requested review from a team June 13, 2024 22:54
@dazuma dazuma added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 17, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 17, 2024
##
# Part of the BatchWrite DSL. This object is passed as a parameter to the block
# passed to {Google::Cloud::Spanner::Client#batch_write}. Use this object to add
# mutation groups to the batch.

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.

I'd say split the last sentence to make it more explicit, and to follow pattern with other files.

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.

Done.

request_options: request_options,
call_options: call_options
results = BatchWriteResults.new response
@last_updated_at = Time.now

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.

Maybe it doesnt need to be done here, but I think we should use Process.clock_gettime. The method idle_since? has a concept of elapsed time and CLOCK_MONOTONIC is better for that case.

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.

You're right, this should be changed to use CLOCK_MONOTONIC. And yes, let's keep that separate. I filed #109 to track it.

#
# @return [::Array<::Integer>]
#
def ok_indexes

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.

successful_mutation_indexes (or indices)? Unless this name was already predetermined.

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.

You're right, that's an awkward name. Changed to successful_indexes.

@dazuma dazuma merged commit f2877ba into main Jun 25, 2024
@dazuma dazuma deleted the batch-write-api branch June 25, 2024 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants