Skip to content
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

Suspend and Resume client APIs #96

Closed
cgillum opened this issue Nov 15, 2022 · 9 comments · Fixed by #104
Closed

Suspend and Resume client APIs #96

cgillum opened this issue Nov 15, 2022 · 9 comments · Fixed by #104
Assignees
Labels
feature-parity This feature is needed for parity with other language SDKs
Milestone

Comments

@cgillum cgillum added the feature-parity This feature is needed for parity with other language SDKs label Nov 15, 2022
@akshaykumars10
Copy link

Hi @cgillum
What should be the ETA for this feature in the java SDK?

@cgillum
Copy link
Member Author

cgillum commented Jan 13, 2023

Adding @kaibocai for visibility.

@cgillum cgillum added this to the v1.1.0 milestone Jan 18, 2023
@kaibocai kaibocai self-assigned this Jan 18, 2023
@cgillum
Copy link
Member Author

cgillum commented Jan 19, 2023

Here's a PR which shows how we added these APIs in the .NET Isolated SDK: microsoft/durabletask-dotnet#97.

One additional thing we'll need in the case of Java is to also add a suspend/resume implementation to TaskOrchestrationExecutor.java, specifically, the processEvent method. The basic steps are outlined in this PR: Azure/durabletask#764. You'll want to focus mostly on the changes to TaskOrchestrationExecutor.cs (same class name as in Java) but also try to implement a similar integration test for validation.

@kaibocai
Copy link
Member

Here's a PR which shows how we added these APIs in the .NET Isolated SDK: microsoft/durabletask-dotnet#97.

One additional thing we'll need in the case of Java is to also add a suspend/resume implementation to TaskOrchestrationExecutor.java, specifically, the processEvent method. The basic steps are outlined in this PR: Azure/durabletask#764. You'll want to focus mostly on the changes to TaskOrchestrationExecutor.cs (same class name as in Java) but also try to implement a similar integration test for validation.

Thanks @cgillum , I believe I will also add missing logics in https://github.com/microsoft/durabletask-sidecar right.

@cgillum
Copy link
Member Author

cgillum commented Jan 20, 2023

No, we don’t use the sidecar project anymore for Functions. There’s no need to change anything there.

@kaibocai
Copy link
Member

Hi @cgillum , so currently all java sdk integration tests are based on durable side car just similar as .NET sdk (seems .NET sdk also skip the test because of missing logics in sidecar microsoft/durabletask-dotnet#97), if we don't use sidecar for the integration tests, are you expecting we spawn up function runtime (just as what we are doing here https://github.com/microsoft/durabletask-java/tree/main/samples-azure-functions) for the testing? Thanks.

@cgillum
Copy link
Member Author

cgillum commented Jan 20, 2023

Oh, interesting. I didn’t realize that any changes to the sidecar were necessary. Maybe it needs to understood the new Suspended enum value? If that’s the case, I recommend we try to update the sidecar to support this because that will make testing easier. My hope is that it’s very little work.

@cgillum
Copy link
Member Author

cgillum commented Jan 20, 2023

Actually, I just realized that I have local changes on my machine to support suspend/resume. I will create a PR for updating the sidecar.

@cgillum
Copy link
Member Author

cgillum commented Jan 20, 2023

Here's the PR for suspend/resume support in the sidecar. I don't have a way to test it yet, unfortunately, so I recommend you clone this PR locally and run it locally (F5 in visual studio, debugging the Microsoft.DurableTask.Sidecar.App project should be good enough) for the testing of the Java work. If you run into issues, feel free to either let me know or investigate/make changes yourself.

https://github.com/microsoft/durabletask-sidecar/pull/18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-parity This feature is needed for parity with other language SDKs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants