Prioritize retrieval of environment variables from IConfiguration instead of directly #1338
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
I've made a couple of changes here:
I noticed while putting this together that the environment variables documentation doesn't include either
DAPR_HTTP_ENDPOINT
norDAPR_GRPC_ENDPOINT
and have created a separate PR to include them.Possible breaking change
But as neither
DAPR_HTTP_ENDPOINT
norDAPR_GRPC_ENDPOINT
are otherwise indicated in the docs to clarify this, I assume it's a bug that today's DaprDefaults will ignore the ports specified inDAPR_HTTP_PORT
orDAPR_GRPC_PORT
if the endpoint is specified (opting to use the endpoint specified with its port instead) and will only use them when the endpoint isn't specified (and defaults instead to "127.0.0.1").I don't think that's necessarily right - that the endpoint should generally specify only the schema and hostname and that if the port is provided, it should be used (instead of potentially inferring the port from the schema, e.g. https => 443 or http => 80). As such, this implementation changes this behavior and will set the endpoint (schema, host and port) and then set the port if also specified.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #1336
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list:
This isn't yet ready to merge as I want to apply the same to the other Dapr clients as well (Jobs, Workflow, etc.) and largely get away from using DaprDefaults where possible.
Also, this presently builds atop the Jobs PR, so it appears a little heavier than it necessarily is.