Skip to content

expose dns load balancing options through c bridge - #1251

Closed
robcao wants to merge 2 commits into
temporalio:mainfrom
robcao:dns-c-bridge
Closed

expose dns load balancing options through c bridge#1251
robcao wants to merge 2 commits into
temporalio:mainfrom
robcao:dns-c-bridge

Conversation

@robcao

@robcao robcao commented May 6, 2026

Copy link
Copy Markdown

What was changed

Added TemporalCoreClientDnsLoadBalancingOptions to the C bridge, so that users of the C bridge can set these options how they need

Why?

Users of the C bridge want to be able to change the dns load balancing options

Checklist

  1. Closes

#1250

  1. How was this tested:

Consumed this change in the .NET SDK: temporalio/sdk-dotnet#676

Verified all tests pass (both cargo test in this repository, and dotnet test in the dotnet-sdk`)

  1. Any docs updates needed?

@Sushisource Sushisource left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@Sushisource
Sushisource enabled auto-merge (squash) May 6, 2026 18:36
@Sushisource

Copy link
Copy Markdown
Member

Looks like this just needs a cargo fmt @robcao

auto-merge was automatically disabled May 6, 2026 18:39

Head branch was pushed to by a user without write access

@robcao

robcao commented May 6, 2026

Copy link
Copy Markdown
Author

woops, done

@jmaeagle99

Copy link
Copy Markdown
Contributor

I think this should drop the enabled fields in favor of breaking the default behavior for a cleaner design such that passing null actually means disabled (instead of default behavior) and push the default behavior up to the lang SDK layers, just like in temporalio/sdk-python#1501. @Sushisource, what do you think?

@robcao

robcao commented May 11, 2026

Copy link
Copy Markdown
Author

I'll be on vacation until June 8, so I created another commit with the alternate approach dropping the enabled field: robcao@497a98e

Feel free to use that instead (or to re-create the change, or I could also pick this up when I return)

@jmaeagle99

Copy link
Copy Markdown
Contributor

Thank you for bringing the issue to our attention and proactively providing a fix.

I ended up reimplementing the change in #1267 without the enabled field and aligned it with the behavior of the other SDKs. I'll close out this PR at this time since it has been superseded.

@jmaeagle99 jmaeagle99 closed this May 14, 2026
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.

3 participants