support continue as new pattern

[kaibocai]

This pr includes:

• add logics for supporting continue as new pattern
• add on end2end test case

refer to #6

[kaibocai]

This method is never invoked, but still pass the test. I am wondering where I made the mistake. @cgillum @davidmrdavid please help.

[cgillum]

What were you expected this code would be needed for? I'm not sure if this is actually needed.

[davidmrdavid]

The Python SDK has something like this here -https://github.com/Azure/azure-functions-durable-python/blob/2f679e9e6507e7c41195fc752c26e215c532e014/azure/durable_functions/models/TaskOrchestrationExecutor.py#L109

In essence, in Python the initialize method helps "clear" the orchestrator's state when a continue_as_new event is processed by the TaskOrchestrationExecutor.

However, in a later comment you clarify that the Continue_as_new is no longer an event type, which is news to me :) . So if this is already being handled in "the backend", then this is certainly not needed

[kaibocai]

when debugging the test, never see a event with this type coming. but the test still pass. @cgillum @davidmrdavid please help.

[cgillum]

This is expected. Only older versions of the Durable Task Framework actually use this event. Newer versions implement this handling directly in the host. We can remove it from the switch statement.

[davidmrdavid]

I see, this is new to me. So is it fair to conclude that, moving forward, OOProc PLs don't need to handle ContinueAsNew history events at all?

[cgillum]

Yes, I think it's fair to ignore this moving forward.

[cgillum]

Glad to see you got this working so quickly! Some feedback:

[cgillum]

Instead of exposing the gRPC types in the public API (OrchestratorService.*), let's just have the user pass in the orchestration instance ID as a String. Something like this:

void sendEvent(String instanceId, String eventName, Object eventData);

The internal implementation of sendEvent can then construct the OrchestrationInstance object.

Also, consider adding a default overload that passes null for eventData.

[cgillum]

This is expected. Only older versions of the Durable Task Framework actually use this event. Newer versions implement this handling directly in the host. We can remove it from the switch statement.

[cgillum]

Per my other feedback let's replace OrchestrationInstanceBuilder.build() with this.instanceId.

[cgillum]

What were you expected this code would be needed for? I'm not sure if this is actually needed.

[cgillum]

nit: fix whitespace here

Suggested change

	public void continueAsNew(Object input, boolean preserveUnprocessedEvents ){
	public void continueAsNew(Object input, boolean preserveUnprocessedEvents) {

[cgillum]

nit: let's remove this extra whitespace.

[cgillum]

I would rephrase this slightly:

Suggested change

	throw new IllegalArgumentException("Argument '" + argName + "' was WhiteSpace.");
	throw new IllegalArgumentException("The argument '" + argName + "' was empty or contained only whitespace.");

[cgillum]

Let's add another integration test that covers this case (unprocessed events). Basically, you should write an orchestration that listens for a single external event and then immediately does ctx.continueAsNew(n + 1). The client should then start an orchestration and, without waiting, immediately raise N external events to the orchestration. For example, 10 events. The client should then wait for the orchestration to complete and verify that the input (and/or output) on the OrchestrationMetadata object is 10, meaning that all 10 external events were processed.

[davidmrdavid]

I agree with Chris' suggestions. Also left some thoughts and questions, mostly for Chris as well :)

[davidmrdavid]

I keep on seeing this version stuff in the actions, which the old out-of-process model did not need. Is this a new requirement, @cgillum?

[cgillum]

The Durable Task Framework has a concept of versions of activities and orchestrations. However, we never exposed it to Durable Functions because I couldn't reason about how to leverage it effectively. I went ahead and carried it forward for the OOProc protocols since I want those to work with DTFx but even in the new SDKs we don't actually use them anywhere. At some point we'll want to spend time thinking about how to implement versioning, but we're not there quite yet.

[davidmrdavid]

I see, this is new to me. So is it fair to conclude that, moving forward, OOProc PLs don't need to handle ContinueAsNew history events at all?

[cgillum]

@kaibocai please sync the latest changes from this PR so that you have my updates locally on your machine. It would be sad if you accidentally force-push and delete my changes. :)

[cgillum]

LGTM!