temporalio / sdk-typescript

Temporal TypeScript SDK
Other
518 stars 101 forks source link

[Feature Request] Flexible workflow and main method naming #228

Closed joebowbeer closed 3 years ago

joebowbeer commented 3 years ago

Is your feature request related to a problem? Please describe.

Ideally one should be able to implement any workflow in any language supported by Temporal. This is important when migrating to a new language, e.g., from PHP to Typescript. However, there are currently some incompatibilities regarding workflow and workflow method naming.

For example, I wanted to provide a Typescript implementation of the game rules in a polyglot snake example, but ran into some issues that required changes in the workflow's client.

See https://github.com/tsurdilo/temporal-polyglot-snake/blob/main/game/src/main/java/io/temporal/snakegame/GameRulesWorkflowInterface.java#L10-L11

As I result, I needed to modify the game code to work with the new Typescript workflow implementation.

Describe the solution you'd like

Please eliminate incompatibilities, or at the least document them and recommend strategies to avoid them.

bergundy commented 3 years ago

I don't think we want to support renaming main (now called execute), I don't see much value in it, I feel like it's simpler to just have this as a convention. In #225 we change workflow registration to be export based instead of file based, you can use any export compatible name for your workflow.

joebowbeer commented 3 years ago

Regarding the execute entrypoint:

Searching for PHP WorkflowMethod annotations, I see a lot of different entrypoint names being used, and I do think they make the code more expressive:

run, handle, handler, exec, execute, greet, greetPeriodically, composeGreeting, createGreeting, bookTrip, transfer, compensate, and so on and so on.

In some of the Java examples the entrypoint name is not only part of the named interface but is also referenced in the code: WorkflowInterface::exec.

I think it would be a rude surprise for someone hoping to migrate to TypeScript to realize that they have to change a lot of legacy workflow interfaces.

My documentation suggestion is meant to avoid this surprise.

Also, I think it would be wise to consider how this might be supported in a future enhancement. In TypeScript this seems to me like a job for Decorators.

bergundy commented 3 years ago

I thought about using decorators with typescript but ruled it out because we want JS to be fully supported in the SDK so we can't use any exclusive TS features. I can't think of an ergonomic way to support custom method names and I'm not convinced it adds much in terms of readability. I think this is one of those cases where we should go with the language constraints. Note that the go SDK doesn't even have an execute method and the prototype rust SDK doesn't either. Also note that there's no real meaning to this method other perhaps for improved readability. You can still define a Workflow interface in Java (or PHP) with a custom method name and use that to call a Workflow in node.

joebowbeer commented 3 years ago

Thanks for the explanation.

At the generic SDK level, that is, not specific to the Node SDK, I think it would be helpful to provide a compatibility matrix showing what can call what, within certain naming restrictions. That would clarify how to code for maximum cross-language compatibility.

Related: #232

bergundy commented 3 years ago

I agree, we should document how to call workflows and activities in other languages, ideally we'd have the full matrix. As mentioned above using execute vs. custom method name is purely superficial, there are no compatibility issues.

I can have the following workflow interface in Java:

@WorkflowInterface
public interface GreetingWorkflow {
    @WorkflowMethod
    void greet(String name);
}

And call it from node via:

type GreetingWorkflow = (name: string) => {
  execute(): Promise<void>;
}

const wf = client.stub<GreetingWorkflow>('greeting', { taskQueue: 'example' });
await wf.execute('joe'); // calls `createGreeting` in Java

Or this node Workflow (using the type above):

export function greetingWorkflow(name: string) {
  return {
    async execute() {
      // ...
    }
  };
}

And call it from Java:

GreetingWorkflow workflow = client.newWorkflowStub(GreetingWorkflow.class, workflowOptions);
workflow.greet("joe") // calls `execute` in node