Native host integration #1213
Native host integration #1213
Conversation
|
Pending release notes update and documentation work. |
There was a problem hiding this comment.
@brettsam will remove this and replace with an issue link to address the logging at initialization
There was a problem hiding this comment.
Consider adding xml doc as part of making this public
There was a problem hiding this comment.
Do we want CancellationToken on these APIs? Or is the cancellation inside StreamingMessage?
Same question for IWorkerClient.SendMessageAsync.
There was a problem hiding this comment.
I think it makes sense for these interfaces to take in a CancellationToken. Will update
src/DotNetWorker.Grpc/NativeHostIntegration/NativeWorkerClient.cs
Outdated
Show resolved
Hide resolved
src/DotNetWorker.Grpc/NativeHostIntegration/NativeWorkerClientFactory.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
What do you think of ValueTask<IWorkerClient> here? Considering one of our implementations is synchronous.
But also not a big deal at all as this is called once per worker I think?
src/DotNetWorker/DotNetWorker.csproj
Outdated
There was a problem hiding this comment.
As with the other csproj, I think the packages can be simplified to just Microsoft.Extensions.Hosting
fabiocav
force-pushed
the
fabiocav/nativehostintegration
branch
from
8abaf8e to
5e23b34
Compare
fabiocav
force-pushed
the
fabiocav/nativehostintegration
branch
from
3faf4a7 to
34e06ec
Compare
|
https://github.com/Azure/azure-functions-language-worker-protobuf/releases/tag/v1.7.1-protofile has the proto changes to support specialization use case. |
| <AssemblyName>Microsoft.Azure.Functions.Worker.Grpc</AssemblyName> | ||
| <RootNamespace>Microsoft.Azure.Functions.Worker.Grpc</RootNamespace> | ||
| <GenerateDocumentationFile>true</GenerateDocumentationFile> | ||
| <MinorProductVersion>7</MinorProductVersion> |
There was a problem hiding this comment.
Should we bring these changes in a minor version update?
| } | ||
|
|
||
| public Task<IWorkerClient> StartClientAsync(IMessageProcessor messageProcessor, CancellationToken token) | ||
| { |
There was a problem hiding this comment.
Should we token.ThrowIfCancellationRequested();?
| { | ||
| // Dispatch and return. | ||
| Task.Run(() => ProcessRequestCoreAsync(request)); | ||
| _ = ProcessRequestCoreAsync(message); |
There was a problem hiding this comment.
We should also update env reload case (line 116) to return capabilities and worker metadata. Can be a follow up PR I can take care of.
There was a problem hiding this comment.
Has the host been updated to support that? yes, we should, but either way, I'm inclined to say this should be a separate change
|
/check-enforcer evaluate |
Managed side of the custom host work introduced in #1212
The code here detects when running in the Functions .NET host and replaces the client to communicate through it.
Pull request checklist
release_notes.md