Add overload to improve the way to call orchestrations, activities, and entities

[hossam-nasr]

Resolves #445 in a non-breaking way:

• Updates df.app.activity() to return a CallActivityInput.
• Updates df.app.orchestration() to return a CallSubOrchestratorInput.
• Adds an overload to context.df.callActivity() and context.df.callActivityWithRetry() to accept a CallActivitInput as first argument instead of a string, while deprecating the old overload
• Adds an overload to context.df.callSubOrchestrator() and context.df.callSubOrchestratorWithRetry() to accept a CallSubOrchestratorInput as first argument instead of a string, while deprecating the old overload
• Adds improvement to entities too, by changing return type of df.app.entity() to RegisterEntityResult, which includes a getEntityId() method that takes a key and returns an EntityId instance (can be passed to context.df.callEntity() and other entity interaction APIs on clients and orchestration contexts.

Before:
Activities/Orchestrations:

const activityName = 'helloActivity';
const subOrchestratorName = 'sayHelloWithActivity';

const orchestrator: OrchestrationHandler = function* (context: OrchestrationContext) {
    const outputs = [];
    outputs.push(yield context.df.callSubOrchestrator(subOrchestratorName, 'Tokyo'));
    outputs.push(yield context.df.callSubOrchestrator(subOrchestratorName, 'Seattle'));
    outputs.push(yield context.df.callSubOrchestrator(subOrchestratorName, 'Cairo'));

    return outputs;
};
df.app.orchestration('durableOrchestrator1', orchestrator);

const sayHelloWithActivityHandler: OrchestrationHandler = function* (context: OrchestrationContext) {
    const input = context.df.getInput();
    return yield context.df.callActivity(activityName, input);
};
 df.app.orchestration(subOrchestratorName, sayHelloWithActivityHandler);

const helloActivityHandler: ActivityHandler = (input: string): string => {
    return `Hello, ${input}`;
};
df.app.activity(activityName, { handler: helloActivityHandler });

Entities:

const entityName = 'counterEntity';

const counterEntityHandler: EntityHandler<number> = (context: EntityContext<number>) => {
    // entity implementation
};
df.app.entity(entityName, counterEntityHandler);

const httpStart: HttpHandler = async (req: HttpRequest, context: InvocationContext): Promise<HttpResponse> => {
    const id: string = req.params.id;
    const entityId: df.EntityId = new df.EntityId(entityName, id);
    const client = df.getClient(context);
    // use entityId for management operations
    await client.signalEntity(entityId, 'add', 1);
    const stateResponse = await client.readEntityState(entityId);
    // ...etc.
};

After:

Orchestrations/Activities:

const orchestrator: OrchestrationHandler = function* (context: OrchestrationContext) {
    const outputs = [];
    outputs.push(yield context.df.callSubOrchestrator(sayHelloWithActivity, 'Tokyo'));
    outputs.push(yield context.df.callSubOrchestrator(sayHelloWithActivity, 'Seattle'));
    outputs.push(yield context.df.callSubOrchestrator(sayHelloWithActivity, 'Cairo'));

    return outputs;
};
df.app.orchestration('durableOrchestrator1', orchestrator);

const sayHelloWithActivityHandler: OrchestrationHandler = function* (context: OrchestrationContext) {
    const input = context.df.getInput();
    return yield context.df.callActivity(helloActivity, input);
};
const sayHelloWithActivity: CallSubOrchestratorInput = df.app.orchestration(
    'sayHelloWithActivity',
    sayHelloWithActivityHandler
);

const helloActivityHandler: ActivityHandler = (input: string): string => {
    return `Hello, ${input}`;
};
const helloActivity: CallActivityInput = df.app.activity('hello', { handler: helloActivityHandler });

Entities:

const counterEntityHandler: EntityHandler<number> = (context: EntityContext<number>) => {
    // entity implementation
};
const counterEntity: RegisterEntityResult = df.app.entity('counterEntity', counterEntityHandler);

const httpStart: HttpHandler = async (req: HttpRequest, context: InvocationContext): Promise<HttpResponse> => {
    const id: string = req.params.id;
    const entityId: df.EntityId = counterEntity.getEntityId(id);
    const client = df.getClient(context);
    // use entityId for management operations
    await client.signalEntity(entityId, 'add', 1);
    const stateResponse = await client.readEntityState(entityId);
    // ...etc.
};

[davidmrdavid]

The PR makes sense to me, but first I have some questions on naming

[ejizba]

The benefits of this change are not as clear-cut as I initially thought they were. Some feedback on the description:

1. I don't think the "Before" examples are accurate. I think most people (including our templates) would put 'sayHelloWithActivity' in a const which is much safer.
2. It bothers me in the "After" example that sayHelloWithActivity is used before it's defined. I know technically it will work as-is, but some linters will flag that as a problem and from a readability perspective I agree with the linters. With a string the order doesn't matter, but with the object it does.
3. I would like us to explore more complex projects where these aren't all in the same file. My gut is telling me that the imports could get complicated now that we depend on the output of registering the handler instead of using a static string. For example - have you ever run into circular import errors? Not sure if they're possible in durable, but they're a huge pain haha

[davidmrdavid]

+1 to (3). Can we do a manual check for how this works across multiple files?

As for whether circular import errors are possible in DF: You definitely could force one. Have 2 Orchestrators in different files and have them spawn each other as sub-orchestrators. I would be curious to see how the runtime handles this.

[hossam-nasr]

Re: 1. Fair enough, I've updated the description.
Re: 2. We could switch the order in the templates, but I agree it is inconvenient.
Re: 3. I've tested this with the following project, and it worked fine:

// helloActivity.ts:
import * as df from 'durable-functions';
import { ActivityHandler } from 'durable-functions';

const helloActivityHandler: ActivityHandler = (input: string): string => {
    return `Hello, ${input}`;
};

export default df.app.activity('helloActivity', { handler: helloActivityHandler });

//

// sayHelloWithActivity.ts:
import * as df from 'durable-functions';
import { OrchestrationContext, OrchestrationHandler } from 'durable-functions';
import helloActivity from './helloActivity';

const sayHelloWithActivityHandler: OrchestrationHandler = function* (context: OrchestrationContext) {
    const input = context.df.getInput();
    return yield context.df.callActivity(helloActivity, input);
};

export default df.app.orchestration('sayHelloWithActivity', sayHelloWithActivityHandler);

// 

// helloSequence.ts:

import * as df from 'durable-functions';
import { OrchestrationContext, OrchestrationHandler } from 'durable-functions';
import sayHelloWithActivity from './sayHelloWithActivity';

const orchestrator: OrchestrationHandler = function* (context: OrchestrationContext) {
    const outputs = [];
    outputs.push(yield context.df.callSubOrchestrator(sayHelloWithActivity, 'Tokyo'));
    outputs.push(yield context.df.callSubOrchestrator(sayHelloWithActivity, 'Seattle'));
    outputs.push(yield context.df.callSubOrchestrator(sayHelloWithActivity, 'Cairo'));

    return outputs;
};
df.app.orchestration('helloSequence', orchestrator);

I'd argue I think it's cleaner to use this style when you have multiple orchestrations/activities across multiple files, and might be cleaner to use constant strings at the top if everything is in one file.

[ejizba]

I feel these patterns are pretty close in functionality, and largely come down to personal preference. In that case, is the downside of overloads really worth the new option? If the new one was clearly better, I'd be fine with the overloads because the downside would be temporary. As it is, I feel like we'll be stuck supporting both overloads indefinitely because there will never be a compelling enough reason to remove the old pattern.

To remind y'all, this is why I try to avoid overloads - it turns a one line error into an ugly five line error:

Without overloads	With overloads

@davidmrdavid @hossam-nasr Are you both confident you like the new pattern better? If yes, I will defer to you guys and I'm fine getting it in. If no, I will remind you that we have plenty of time to tinker with this design since we're doing it in a non-breaking way. We can easily ship this feature any time in the future regardless of announcement dates

[davidmrdavid]

@ejizba: Thanks for the concrete example. I'm confident that, across all DF SDKs, we're looking to support safer ways of calling Activities/Sub-Orchestrators/Entities by providing alternatives to this string-based parameter. I know I've been asked to do something equivalent to this in Python's new progmodel, although overloads there a bit a nicer in error cases.

Yes, we have time to tinker with this design and it appears we would be ok to merge this at a later date, so I'm no rush. I think this would be something I'd love to demo, among other bits, to the DF team. That would help us be in the same page about the tradeoffs of the feature.

[hossam-nasr]

I don't have strong feelings about either merging this now (and including it in preview), or punting until later, after/during preview. I will defer to @davidmrdavid on this one. Let me know if you think we should have this PR in before preview and I'll happily rebase the branch so we can merge.

[hossam-nasr]

Looks like we've decided to table this for preview, and we'll revisit this later. This is non-breaking so we can always add it after preview before GA, at GA, or even after GA. We already have two issues tracking this: #445 and #418, so I'm closing this PR