💥 Enable TLS if api key provided #1229
Conversation
temporalio/service.py
Outdated
| # Enable TLS by default when API key is provided and tls not explicitly set | ||
| if self.tls is None: | ||
| self.tls = self.api_key is not None |
There was a problem hiding this comment.
I know we default identity here, but I would suggest we don't do this derived default here. I think tls should remain None if they look at their config (because it is the config they created/provided after all) and we only apply the default when building the bridge config.
If necessary, I can show an admittedly contrived situation where setting this config to a value they didn't set can cause a problem if they try to reuse it, but it would be a rare case.
There was a problem hiding this comment.
Agreed, mutating the config seems weird versus the config just meaning something different when provided. Either in the worker or the bridge worker etc
cretz
left a comment
cretz
left a comment
There was a problem hiding this comment.
LGTM. We need to remember to call this out in release notes quite big. Would also make sure @tconley1428 and/or @VegetarianOrc approve before merging.
What was changed
DWISOTT
Part of Assume TLS enabled if API key provided in SDKs features#687
How was this tested:
Unit tests
Any docs updates needed?
Maybe?