Skip to content

Use IDurableClient for gRPC OOP languages#2544

Merged
jviau merged 3 commits intodevfrom
jviau/grpc-client
Aug 22, 2023
Merged

Use IDurableClient for gRPC OOP languages#2544
jviau merged 3 commits intodevfrom
jviau/grpc-client

Conversation

@jviau
Copy link
Copy Markdown
Contributor

@jviau jviau commented Aug 14, 2023

This PR changes LocalGrpcListener to use IDurableClient in select places as part of its implementation. Previously we used IOrchestrationService directly to implement the gRPC calls. However, this means logic that is part of TaskHubClient is not ran. This is not a problem today, but it will be an issue for our distributed tracing improvements. Our tracing work has logic in TaskHubClient that is essential for it to all work.

This PR also introduces more orchestrations to the smoke test project for easier manual testing of multiple orchestration patterns.

Issue describing the changes in this PR

resolves an issue for distributed tracing work

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation PR is ready to merge and referenced in pending_docs.md
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests)
  • My changes do not require any extra work to be leveraged by OutOfProc SDKs
    • Otherwise: That work is being tracked here: #issue_or_pr_in_each_sdk
  • My changes do not change the version of the WebJobs.Extensions.DurableTask package
    • Otherwise: major or minor version updates are reflected in /src/Worker.Extensions.DurableTask/AssemblyInfo.cs
  • My changes do not add EventIds to our EventSource logs
    • Otherwise: Ensure the EventIds are within the supported range in our existing Windows infrastructure. You may validate this with a deployed app's telemetry. You may also extend the range by completing a PR such as this one.

@jviau jviau requested a review from bachuv August 14, 2023 17:56
Copy link
Copy Markdown
Collaborator

@bachuv bachuv left a comment

Choose a reason for hiding this comment

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

Looks good to me! I would like to address the comments from Chris before approving though.

@jviau jviau requested review from bachuv and cgillum August 21, 2023 21:45
Copy link
Copy Markdown
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.

High-level question: this seems like a pretty obvious thing to do, and I don't recall why we didn't consider it originally. Do you see any downsides with using IDurableClient instead of IOrchestrationService?

@jviau
Copy link
Copy Markdown
Contributor Author

jviau commented Aug 22, 2023

High-level question: this seems like a pretty obvious thing to do, and I don't recall why we didn't consider it originally. Do you see any downsides with using IDurableClient instead of IOrchestrationService?

No, I don't see any. I cannot say exactly why it wasn't done this way to start (I didn't write it). I am guessing maybe it was because the GrpcDurableTaskClient was written with IOrchestrationService, so this was done the same way without thinking too deeply about it.

@jviau jviau merged commit e74c4fb into dev Aug 22, 2023
@jviau jviau deleted the jviau/grpc-client branch August 22, 2023 16:54
jviau added a commit that referenced this pull request Sep 13, 2023
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.

3 participants