-
Notifications
You must be signed in to change notification settings - Fork 401
feat: Spanner non-awaiting DDL #15280
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
Changes from 3 commits
1aa3b4f
c299d51
5752bb3
3c984c9
901a44e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
|
|
||
| using Google.Api.Gax; | ||
| using Google.Cloud.Spanner.V1; | ||
| using Google.LongRunning; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Data; | ||
|
|
@@ -513,6 +514,39 @@ public long ExecutePartitionedUpdate() => | |
| public Task<long> ExecutePartitionedUpdateAsync(CancellationToken cancellationToken = default) => | ||
| CreateExecutableCommand().ExecutePartitionedUpdateAsync(cancellationToken); | ||
|
|
||
| /// <summary> | ||
| /// Executes this command as DDL. The command must contain one or more DDL statements; | ||
| /// <see cref="SpannerConnection.CreateDdlCommand(string, string[])"/> for details. | ||
| /// </summary> | ||
| /// <param name="pollUntilCompleted"> | ||
| /// If true, the returned task will wait for the DDL operation to finish execution on Spanner. | ||
| /// Otherwise, the returned task will finish as soon as the DDL operation has successfully been started on | ||
| /// Spanner, and the caller can monitor the progress of the long-running operation. | ||
olavloite marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| /// </param> | ||
| /// <returns> | ||
| /// A reference to the long-running operation that was started for the DDL statement(s). | ||
| /// Note: The operation is null for DropDatabase commands. | ||
| /// </returns> | ||
| public Operation ExecuteDdl(bool pollUntilCompleted = false) => | ||
olavloite marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Task.Run(() => ExecuteDdlAsync(pollUntilCompleted, _synchronousCancellationTokenSource.Token)).ResultWithUnwrappedExceptions(); | ||
|
|
||
| /// <summary> | ||
| /// Executes this command as DDL. The command must contain one or more DDL statements; | ||
| /// <see cref="SpannerConnection.CreateDdlCommand(string, string[])"/> for details. | ||
| /// </summary> | ||
| /// <param name="pollUntilCompleted"> | ||
| /// If true, the returned task will wait for the DDL operation to finish execution on Spanner. | ||
| /// Otherwise, the returned task will finish as soon as the DDL operation has successfully been started on | ||
| /// Spanner, and the caller can monitor the progress of the long-running operation. | ||
olavloite marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| /// </param> | ||
| /// <param name="cancellationToken">An optional token for canceling the call.</param> | ||
| /// <returns> | ||
| /// A reference to the long-running operation that was started for the DDL statement(s). | ||
| /// Note: The operation is null for DropDatabase commands. | ||
| /// </returns> | ||
| public Task<Operation> ExecuteDdlAsync(bool pollUntilCompleted = false, CancellationToken cancellationToken = default) => | ||
|
||
| CreateExecutableCommand().ExecuteDdlAsync(pollUntilCompleted, cancellationToken); | ||
|
|
||
| /// <summary> | ||
| /// Creates an executable command that captures all the necessary information from this command. | ||
| /// </summary> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding this if in two places, let's have this method return the un-polled operation and the wrapping ones poll until completion if they need to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that really possible in this case, considering the fact that the two methods return two different operation types:
Operation<Database, CreateDatabaseMetadata>Operation<Empty, UpdateDatabaseDdlMetadata>(Type non-generic
Operationclass cannot be used for polling)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the goal is to remove duplication might I instead suggest a a generic local function to centralize the polling and error-handling logic (at the bottom of this method):
If we go the other way and use the returned
Operationto poll later, the calling method will add extra overhead because it needs to establish a new channel since this method shuts down the one it creates in thefinallyblock.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes +1 to Robert's comment, I hadn't noticed the channel being closed.