-
Notifications
You must be signed in to change notification settings - Fork 94
update samples to use environment configuration for client #233
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
Conversation
a92c84f to
df18612
Compare
activity_worker/activity_worker.py
Outdated
| config = ClientConfig.load_client_connect_config( | ||
| config_file=str(get_temporal_config_path()) | ||
| ) | ||
|
|
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.
Pedantic, but probably don't need this blank line in every sample
a3daf44 to
136e3fc
Compare
d9c0f98 to
fd5b254
Compare
cretz
left a comment
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.
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.
* Update SDK version to 1.18 * Update openai breaking change
fccf464 to
f2dd3d3
Compare
cretz
left a comment
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.
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 |
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.
Closes [Feature Request] Environment configuration #178
How was this tested:
Existing tests.
Any docs updates needed?
No