Skip to content

Conversation

@THardy98
Copy link
Contributor

@THardy98 THardy98 commented Aug 12, 2025

What was changed

Update samples to use environment configuration for clients.

Why?

Client environment configuration is intended as the preferred way to configure clients going forward.

  1. Closes [Feature Request] Environment configuration #178

  2. How was this tested:
    Existing tests.

  3. Any docs updates needed?
    No

@THardy98 THardy98 requested a review from a team as a code owner August 12, 2025 02:59
@THardy98 THardy98 force-pushed the update_samples_env_config branch 3 times, most recently from a92c84f to df18612 Compare August 12, 2025 04:26
config = ClientConfig.load_client_connect_config(
config_file=str(get_temporal_config_path())
)

Copy link
Member

Choose a reason for hiding this comment

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

Pedantic, but probably don't need this blank line in every sample

@THardy98 THardy98 requested a review from cretz August 12, 2025 14:46
@THardy98 THardy98 force-pushed the update_samples_env_config branch from a3daf44 to 136e3fc Compare August 13, 2025 16:06
@THardy98 THardy98 force-pushed the update_samples_env_config branch from d9c0f98 to fd5b254 Compare August 28, 2025 15:09
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

Looks great to me. Holding off approving for same reason as at temporalio/samples-go#421 (review), not sure we want to tell everyone in all samples to use this yet. But an equivalent of temporalio/samples-go#420 that has advanced env config + just updating one may make sense for now.

@THardy98 THardy98 force-pushed the update_samples_env_config branch from fccf464 to f2dd3d3 Compare November 10, 2025 18:40
@THardy98 THardy98 requested a review from cretz November 10, 2025 18:46
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

I trust we at least ran a sample or two to make sure they still work

@THardy98
Copy link
Contributor Author

I trust we at least ran a sample or two to make sure they still work

Yup ran a basic sample, nexus sample (sets namespace as well), custom metric/data converter (sets runtime/data converter) and context propagation (sets interceptor). Works fine

@THardy98 THardy98 merged commit ff0f134 into main Nov 11, 2025
11 checks passed
@THardy98 THardy98 deleted the update_samples_env_config branch November 11, 2025 02:52
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.

[Feature Request] Environment configuration

4 participants