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

Workflow.newExternalWorkflowStub #2180

Open
mfateev opened this issue Aug 11, 2024 · 4 comments
Open

Workflow.newExternalWorkflowStub #2180

mfateev opened this issue Aug 11, 2024 · 4 comments

Comments

@mfateev
Copy link
Member

mfateev commented Aug 11, 2024

Expected Behavior

The following works when using Workflow Client:

public interface Retryable {
    @SignalMethod
    void retryNow();
}

@WorkflowInterface
public interface FileProcessingWorkflow extends Retryable {

    @WorkflowMethod
    String processFile(Arguments args);

    @QueryMethod(name="history")
    List<String> getHistory();

    @QueryMethod
    String getStatus();

    @SignalMethod
    void abandon();
}

@WorkflowInterface
public interface MediaProcessingWorkflow extends Retryable {

    @WorkflowMethod
    String processBlob(Arguments args);
}

Retryable r1 = client.newWorkflowStub(Retryable.class, firstWorkflowId);
Retryable r2 = client.newWorkflowStub(Retryable.class, secondWorkflowId);
r1.retryNow();
r2.retryNow();

The same should work when signaling workflow from an external workflow:

Workflow.newExternalWorkflowStub(Retryable.class, firstWorkflowId);

Retryable r1 = Workflow.newExternalWorkflowStub(Retryable.class, firstWorkflowId);
Retryable r2 = Workflow.newExternalWorkflowStub(Retryable.class, secondWorkflowId);
r1.retryNow();
r2.retryNow();

Actual Behavior

The newExternalWorkflowStub fails with:

Caused by: java.lang.IllegalArgumentException: Missing required @WorkflowInterface annotation: interface ...Retryable
	at io.temporal.common.metadata.POJOWorkflowInterfaceMetadata.newInstanceInternal(POJOWorkflowInterfaceMetadata.java:179)
	at io.temporal.common.metadata.POJOWorkflowInterfaceMetadata.newInstance(POJOWorkflowInterfaceMetadata.java:108)
	at io.temporal.internal.sync.ExternalWorkflowInvocationHandler.<init>(ExternalWorkflowInvocationHandler.java:43)
	at io.temporal.internal.sync.WorkflowInternal.newExternalWorkflowStub(WorkflowInternal.java:412)
	at io.temporal.workflow.Workflow.newExternalWorkflowStub(Workflow.java:204)
@cretz
Copy link
Member

cretz commented Aug 12, 2024

Arguably it shouldn't even work on the client. It'd make one wonder why a @WorkflowInterface is even needed ever if we can just assume it's there. In Python and .NET at least, we require the client class passed in to have this (even if it's a base class, to signify user intent). But I can understand if Java we don't want that.

@mfateev
Copy link
Member Author

mfateev commented Aug 13, 2024

@WorkflowInterface defines a workflow type. There is no "Retryable" workflow type to register.

@cretz
Copy link
Member

cretz commented Aug 14, 2024

I was under the impression the annotation is also meant to be required for caller use. I assume with this issue, we should accept basically any interface in Java including standard library ones (and that the client side already does)? I assume that it's at method call time on the proxy where we will fail for any non-signal/update/query-annotated method? It could be argued (and how we did it in .NET/Python) that if you're willing to define methods w/ Temporal annotations in an interface, you should mark that interface as one capable of being used as a workflow stub via the @Workflow annotation.

But I understand if in Java we expect methods at any level to have annotations but not classes. And of course there's value in consistency with client side. Can disregard my concern.

@mfateev
Copy link
Member Author

mfateev commented Aug 14, 2024

The WorkflowClient.newWorkflowStub call validates that only methods that are annotated with signal/update/query are allowed at the interface.

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

No branches or pull requests

2 participants