Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat[Spanner]: Add samples for new features #2550

Merged

Conversation

amanda-tarafa
Copy link
Member

Tests won't pass until googleapis/google-cloud-dotnet#12164 is merged and release. I'm currently running them locally.

@amanda-tarafa amanda-tarafa requested a review from jskeet March 20, 2024 08:19
@amanda-tarafa amanda-tarafa requested review from a team as code owners March 20, 2024 08:19
Copy link

snippet-bot bot commented Mar 20, 2024

Here is the summary of changes.

You are about to add 2 region tags.

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

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Mar 20, 2024
jskeet
jskeet previously approved these changes Mar 20, 2024
Copy link
Member

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

Nice and simple :)

@amanda-tarafa
Copy link
Member Author

@jskeet I've re-requested review as I fixed a copyright year and the previous approval was lost.

jskeet
jskeet previously approved these changes Mar 20, 2024
@amanda-tarafa amanda-tarafa force-pushed the spanner-directed-reads branch 2 times, most recently from 3e39c6a to 79e70c2 Compare April 4, 2024 23:20
@amanda-tarafa
Copy link
Member Author

amanda-tarafa commented Apr 4, 2024

The first commit here is from #2588 and will be rebased away once that's merged. We need it in the meantime so these tests run.

We'll do dependency updates and all sample in this PR in separate commits, it's all easier to manage.

@amanda-tarafa amanda-tarafa changed the title feat[Spanner]: Add directed reads sample feat[Spanner]: Add samples for new features Apr 4, 2024
@amanda-tarafa
Copy link
Member Author

@jskeet Here are the dependency updates for Spanner samples and the samples themselves, you had already reviewed one sample but not the other.
And also, when updating the dependencies I had to fix a test. PTAL, thanks!

return rowsInserted;
});

Console.WriteLine("Data inserted.");
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how this is compiling - isn't this unreachable code? (We've got a return statement just above it.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Well it's compiling also in local. But you are right it is unreachable, I'll remove it (and I'll check from where I copied it.)

jskeet
jskeet previously approved these changes Apr 5, 2024
- Fix a sample that was not using the intendend transaction. Caught because of transaction inlining.
- Use the new ambient transaction methods as the existing ones are now deprecated.
@amanda-tarafa
Copy link
Member Author

Failures are unrelated and being tracked in #1623 . Force merging.

@amanda-tarafa amanda-tarafa merged commit cd0cf06 into GoogleCloudPlatform:main Apr 5, 2024
7 of 8 checks passed
@amanda-tarafa amanda-tarafa deleted the spanner-directed-reads branch April 5, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants