-
Notifications
You must be signed in to change notification settings - Fork 188
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
[Enhanced HTTP] Add handling for IActionResult #1475
Conversation
extensions/Worker.Extensions.Http.AspNetCore/src/HttpUriProvider.cs
Outdated
Show resolved
Hide resolved
@@ -31,7 +31,7 @@ public static IFunctionsWorkerApplicationBuilder UseAspNetCoreIntegration(this I | |||
|
|||
builder.Services.AddSingleton<IHttpCoordinator, DefaultHttpCoordinator>(); | |||
|
|||
var port = Utilities.GetUnusedTcpPort().ToString(); |
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.
wasn't being used
@@ -27,6 +30,13 @@ public async Task Invoke(FunctionContext context, FunctionExecutionDelegate next | |||
|
|||
await next(context); | |||
|
|||
if (context.GetInvocationResult()?.Value is IActionResult actionResult) | |||
{ | |||
ActionContext actionContext = new ActionContext(httpContext, httpContext.GetRouteData(), new ActionDescriptor()); |
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.
Do we have plans to fill out ActionDescriptor
?
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.
Discussed at sync yesterday and no, we do not. Looking at similar code in the host side and we do not fill it out either.
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.
If we don't do it in host code I am fine leaving this empty then.
I do wonder what functionality we are excluding here? ActionDescriptor
essentially holds a breakdown the MVC endpoint that is being ran. We should consider populating it in the future.
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.
There's no action associated with the context and we have a terminating middleware, no action selection will be performed. As @satvu pointed out, that's the same pattern used by the host.
Issue describing the changes in this PR
resolves #issue_for_this_pr
Pull request checklist
release_notes.md
Additional information
Additional PR information