Use default host and port for lambdaconfig if routeconfig endpoint is not specified #1941
Conversation
| { | ||
| // TODO: Handle routeConfig.Endpoint to null and use the settings versions of runtime. | ||
| var endpoint = routeConfig.Endpoint ?? $"http://{settings.Host}:{settings.Port}"; |
There was a problem hiding this comment.
I am assuming to use http here, since settings.Host is only defined as localhost
There was a problem hiding this comment.
Can we rename Port to LambdaEmulatorPort to avoid future ambiguous confusions.
There was a problem hiding this comment.
sure. same thing with host then too? LambdaEmulatorHost?
There was a problem hiding this comment.
Yes, those names made sense when there was only a single web process.
There was a problem hiding this comment.
i updated in latest version to update the variable names and similar
There was a problem hiding this comment.
looks like i need to rebase with your changes. so i will do that now
ghost
added
the
Release Not Needed
ghost
marked this pull request as ready for review
ghost
force-pushed
the
gcbeatty/routeconfignull
branch
from
2938b9a to
0cea399
Compare
ghost
force-pushed
the
gcbeatty/routeconfignull
branch
from
0cea399 to
9fdd63e
Compare
| @@ -97,7 +96,6 @@ public async Task TestLambdaToUpperRest() | |||
| finally | |||
| { | |||
| await cancellationTokenSource.CancelAsync(); | |||
There was a problem hiding this comment.
this is already callled in DisposeAsync which is why i removed it here
|
latest update has rebased changes. but the existing tests (along with the new one) is still flaky. i will take a deeper look to see if i can figure out whats going on and possibly update in this PR |
Issue #, if available: DOTNET-7934
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.