Skip to content

Conversation

@rajatbhatta
Copy link
Contributor

@rajatbhatta rajatbhatta commented Jun 20, 2023

Note: Github checks are expected to fail until #2520 is released, and a new libraries-bom version is out.

@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: spanner Issues related to the googleapis/java-spanner API. samples Issues that are directly related to samples. labels Jun 20, 2023
@rajatbhatta rajatbhatta marked this pull request as ready for review June 20, 2023 07:11
@rajatbhatta rajatbhatta requested a review from a team as a code owner June 20, 2023 07:11
@snippet-bot
Copy link

snippet-bot bot commented Jun 20, 2023

Here is the summary of changes.

You are about to add 1 region tag.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@rajatbhatta rajatbhatta changed the title samples: add support for BatchWriteAtleastOnce samples: add support for BatchWriteAtLeastOnce Aug 23, 2023
@arpan14 arpan14 closed this Oct 3, 2023
@arpan14 arpan14 reopened this Oct 3, 2023
@arpan14 arpan14 added the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 3, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 3, 2023
@gcf-owl-bot gcf-owl-bot bot requested a review from a team as a code owner October 3, 2023 08:51
* Assume DDL for the underlying database:
* <pre>{@code
* CREATE TABLE Singers (
* SingerId INT64 NOT NULL,
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: add a couple of spaces for indentation

projectId, instanceId, databaseId));
assertTrue(
out.contains("have been applied with commit timestamp")
|| out.contains("could not be applied with error code"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't expect an error here, do we? So in that case I think we should let the test case fail with the error instead of letting the test pass here.

If it is possible that the call fails with a transient error (e.g. UNAVAILABLE or ABORTED), then I think that the sample should show how customers should handle that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I agree we should exclude out.contains("could not be applied with error code") and let the test case fail. Modified this.

@arpan14 arpan14 added the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 3, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 3, 2023
@arpan14 arpan14 merged commit 7be4df1 into googleapis:main Oct 3, 2023
@arpan14 arpan14 deleted the batch-write-sample branch October 3, 2023 12:25
surbhigarg92 pushed a commit to surbhigarg92/java-spanner that referenced this pull request Oct 5, 2023
* samples: add support for BatchWriteAtleastOnce

* samples: add support for BatchWriteAtleastOnce

* samples: add support for BatchWriteAtleastOnce

* modify sample based on new design

* remove host

* chore: review comments.

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* chore: fix review comments.

---------

Co-authored-by: Arpan Mishra <akmish3@gmail.com>
Co-authored-by: Arpan Mishra <arpanmishra@google.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the googleapis/java-spanner API. samples Issues that are directly related to samples. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants