Skip to content

Conversation

@kim
Copy link
Contributor

@kim kim commented Sep 24, 2025

Allows the SpacetimeDBClient to be configured with or without confirmed reads. Like for TypeScript, the parameter is optional -- the server chooses the default if not set explicitly.

Expected complexity level and risk

1

Testing

I don't actually know what I'm doing 😅

Allows the SpacetimeDBClient to be configured with or without confirmed
reads. Like for [TypeScript], the parameter is optional -- the server
chooses the default if not set explicitly.

[TypeScript]: #3247
@kim kim requested a review from rekhoff September 24, 2025 12:12
kim added a commit that referenced this pull request Sep 24, 2025
For consistency with [TypeScript] and [C#].
If `DbConnectionBuilder::with_confirmed_reads` is not called, don't set
the parameter on the connection URL, so that the server can choose the
default.

[TypeScript]: #3247
[C#]: #3282
github-merge-queue bot pushed a commit that referenced this pull request Sep 24, 2025
For consistency with [TypeScript] and [C#].

If `DbConnectionBuilder::with_confirmed_reads` is not called, don't set
the parameter on the connection URL, so that the server can choose the
default.

[TypeScript]: #3247
[C#]: #3282

# Expected complexity level and risk

1
@bfops bfops added the release-any To be landed in any release window label Sep 29, 2025
@kim kim requested a review from JasonAtClockwork November 13, 2025 07:28
Copy link
Contributor

@JasonAtClockwork JasonAtClockwork left a comment

Choose a reason for hiding this comment

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

LGTM - when we improve testing in the future we may want to include confirmation of this flag being set and read.

@JasonAtClockwork JasonAtClockwork dismissed their stale review November 17, 2025 20:06

Mistake on options

Copy link
Contributor

@JasonAtClockwork JasonAtClockwork left a comment

Choose a reason for hiding this comment

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

LGTM - when we improve testing in the future we may want to include confirmation of this flag being set and read.

I did test this against the regression tests with and without and had no troubles.

@kim kim enabled auto-merge November 18, 2025 08:32
@kim kim added this pull request to the merge queue Nov 18, 2025
Merged via the queue into master with commit fa2e9b3 Nov 18, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-any To be landed in any release window

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants