Skip to content

Extract DurableTask.Core shims to common Worker package#55

Merged
jviau merged 3 commits intomicrosoft:mainfrom
jviau:common-worker-layer
Oct 4, 2022
Merged

Extract DurableTask.Core shims to common Worker package#55
jviau merged 3 commits intomicrosoft:mainfrom
jviau:common-worker-layer

Conversation

@jviau
Copy link
Member

@jviau jviau commented Oct 3, 2022

This PR extracts the shims to Microsoft.DurableTask.Worker and allows for public usage of them via DurableTaskShimFactory abstraction. This change also involved some other refactors:

  1. Replaced DurableTask.Core "ParentInstance" usage with our own type.
  2. Refactoring of DurableTaskGrpcWorker and its builder to leverage the shim factory.
  3. Moving FuncTaskOrchestrator to worker package (from abstractions).
  4. Renaming of OrchestrationRunner to GrpcOrchestrationRunner (as it is gRPC specific).

@jviau jviau requested a review from cgillum October 3, 2022 23:23
object? input = this.invocationContext.DataConverter.Deserialize(rawInput, this.implementation.InputType);

this.wrapperContext = new(innerContext, this.name, this.workerContext, this.runtimeState, input);
ILogger contextLogger = this.invocationContext.LoggerFactory.CreateLogger("Microsoft.DurableTask");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously we encapsulated the creation of loggers inside SdkUtils.GetLogger(loggerFactory). Is there a reason to not do that here?

Copy link
Member Author

@jviau jviau Oct 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't have SdkUtils here. I also plan on refactoring that later. We should not be using a single category, but rather multiple categories. We just have to make sure they all start with Microsoft.DurableTask. So this one I actually want to change to .CreateLogger<TaskOrchestrationContext>() (in another PR).

By using multiple categories we will allow for more granular log filtering - if customer so decides. But they can still filter on "Microsoft.DurableTask" to cover all of them.

Copy link
Member

@cgillum cgillum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor comment note, but otherwise LGTM! It's really nice to see the structure of the project take a more mature shape like this.

@jviau jviau merged commit cb108fc into microsoft:main Oct 4, 2022
@jviau jviau deleted the common-worker-layer branch October 4, 2022 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants