Skip to content

chore(efcore): implement sync versions of Retry methods#781

Merged
olavloite merged 4 commits into
mainfrom
efcore-sync-retry-methods
Jun 30, 2026
Merged

chore(efcore): implement sync versions of Retry methods#781
olavloite merged 4 commits into
mainfrom
efcore-sync-retry-methods

Conversation

@olavloite

Copy link
Copy Markdown
Collaborator

This is the first step in a multi-step change to natively support synchronous versions of the Retry methods for aborted transactions. This step adds support for a sync version of the Retry method for DML and Batch DML statements.

This is the first step in a multi-step change to natively support synchronous versions
of the Retry methods for aborted transactions. This step adds support for a sync version
of the Retry method for DML and Batch DML statements.
@olavloite olavloite requested review from a team as code owners June 29, 2026 13:13

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces synchronous Retry methods to the IRetriableStatement interface and its implementations, along with a comprehensive suite of unit tests to verify their behavior. The review feedback suggests explicitly setting the transaction on the command in FailedBatchDmlStatement.Retry for consistency, and simplifying the exception assertions in the unit tests by using xUnit's Assert.Throws<T> instead of manual try-catch blocks.

@olavloite

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces synchronous Retry methods to the IRetriableStatement interface and its various implementations (FailedBatchDmlStatement, FailedDmlStatement, RetriableBatchDmlStatement, RetriableDmlStatement, and SpannerDataReaderWithChecksum), along with corresponding unit tests. The review feedback highlights that the timeoutSeconds parameter is currently ignored across all these new Retry implementations, and recommends setting the CommandTimeout property on the underlying commands before execution to ensure timeouts are respected.

@olavloite

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces synchronous retry support to the IRetriableStatement interface and its implementations (FailedBatchDmlStatement, FailedDmlStatement, RetriableBatchDmlStatement, RetriableDmlStatement, and SpannerDataReaderWithChecksum). It also adds a comprehensive suite of unit tests in RetriableStatementsTests.cs to verify the behavior of these synchronous retries under various success and failure scenarios. I have no feedback to provide as there are no review comments to assess.

@olavloite olavloite merged commit 223522d into main Jun 30, 2026
24 checks passed
@olavloite olavloite deleted the efcore-sync-retry-methods branch June 30, 2026 14:24
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.

2 participants