Skip to content

Conversation

@javier-aliaga
Copy link

Issue describing the changes in this PR

Move integration test to use durabletask-go as sidecar instead of the actual user one

resolves #issue_for_this_pr

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes are added to the CHANGELOG.md
  • I have added all required tests (Unit tests, E2E tests)

Additional information

Additional PR information

@javier-aliaga javier-aliaga force-pushed the use-dapr-durabletask-go-sidecar-ci branch from 1769cee to d7bd353 Compare July 2, 2025 10:53
Signed-off-by: Javier Aliaga <[email protected]>
@javier-aliaga javier-aliaga force-pushed the use-dapr-durabletask-go-sidecar-ci branch from d7bd353 to 864f62c Compare July 2, 2025 10:59
ref: upgrade-dockerfile
path: durabletask-sidecar

# TODO: Move the sidecar into a central image repository
Copy link
Author

Choose a reason for hiding this comment

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

Do we want this or are we happy this way?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@javier-aliaga I get the idea.. but it looks like durabletask-go sidecar is not fully compatible with all the features in the java implementation right?

Copy link
Author

Choose a reason for hiding this comment

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

exactly, there are things we do not support

Signed-off-by: Javier Aliaga <[email protected]>
@javier-aliaga javier-aliaga marked this pull request as ready for review July 2, 2025 12:15
assertThrows(TimeoutException.class, () -> client.waitForInstanceStart(instanceId, Duration.ofSeconds(2)));
var instanceId = UUID.randomUUID().toString();
Thread thread = new Thread(() -> {
client.scheduleNewOrchestrationInstance(orchestratorName, null, instanceId);
Copy link
Author

Choose a reason for hiding this comment

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

this call is blocking until it finishes

@salaboy salaboy self-requested a review July 2, 2025 12:53

DurableTaskClient client = new DurableTaskGrpcClientBuilder().build();
try (worker; client) {
client.createTaskHub(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@javier-aliaga why is this removed?

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

the main problem with all this is that in our implementation we do no support all the features the library supports and I am not sure we want to

Copy link
Author

Choose a reason for hiding this comment

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

but in the backend implementation what it does is initialize the persistence. https://github.com/dapr/durabletask-go/blob/1cae3eb4b56b1970ea0bfca29015c0e2add24781/backend/sqlite/sqlite.go#L99


DurableTaskClient client = new DurableTaskGrpcClientBuilder().build();
try (worker; client) {
client.createTaskHub(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@javier-aliaga why is this removed?

Copy link
Collaborator

@cicoyle cicoyle left a comment

Choose a reason for hiding this comment

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

One comment then Im mostly good with this PR. Im ok disabling the tests we dont support for now, but can you open issues in the java-sdk and add the durabletask label so we dont forget to come back to it in the future?

@javier-aliaga
Copy link
Author

I added an issue dapr/java-sdk#1438

@cicoyle cicoyle merged commit 642161a into dapr:main Jul 3, 2025
2 checks passed
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