updates_do_not_block_continue_as_new

[dandavison]

Adds a new feature that confirms the behavior when a client sends an update to a workflow that is continuing-as-new. Specifically, the test guarantees that the update is admitted while the WFT is in-flight. This confirms that:

1. Updates do not block continue as new (unlike signals)
2. An update that is admitted while the workflow is continuing as new lands on the post-CAN workflow run.

Only implemented for Python so far; let's confirm the approach before extending implementation to other languages.

[cretz]

This is a thread-blocking call in asyncio. Can you use https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_in_executor? Would also request on the in-workflow wait but that makes a bit less sense since blocking the workflow event loop is the goal.

[dandavison]

This is a thread-blocking call in asyncio.

Yes that's deliberate, could you read the docstring at the top of the file and let me know if you agree with the strategy?

[dandavison]

Neither run_in_executor nor wait_condition can be used here since the purpose is to block the event loop in order to hold up WFT processing.

EDIT: Ignore, on re-reading I see you were explicitly not referring to the thread-blocking in workflow code.

[cretz]

Suggested change

	first_run_wft_is_in_progress.wait()
	asyncio.get_running_loop().run_in_executor(None, first_run_wft_is_in_progress.wait)

^ That is the safe way to make a thread-blocking call like this. You don't want to block the primary event loop (blocking the workflow's event loop is different). Blocking the primary event loop prevents all other asyncio systems in the process from running.

[dandavison]

I believe the code is doing this correctly already. This entire coroutine is run in a new event loop in a new thread. See line 93 below:

await asyncio.to_thread(lambda: asyncio.run(_check_result()))

[cretz]

Ah, I see, then I think this can bring up a different issue: our client is not safe for running in separate event loops (because it is not safe for running in separate threads)

[dandavison]

Ah, right. Modified to just do the thread-blocking synchronization itself in a new thread, using asyncio.to_thread:

await asyncio.to_thread(first_run_wft_is_in_progress.wait)

[cretz]

I think it'd be worth two extra assertions: 1) that the update event is not in the history of the pre-CAN run, and 2) that the update is in the history of the post-CAN run

[dandavison]

Good call, added these.

[cretz]

I do fear with single-language features there is no incentive or even reminder to do these in other languages. I wonder if we should track the work anywhere.

[dandavison]

I do fear with single-language features there is no incentive or even reminder to do these in other languages.

It's not clear that this should be a feature test at all, so didn't seem any point in doing it in multiple langauges. If the retries are done by Frontend / History gRPC interceptors / clients as they are supposed to be then, while it's fine to add it as an integration test in SDKs, it's unclear that there's a need for a feature test across SDKs.

I wonder if we should track the work anywhere.

One possibility would be to set up a nag of some sort that finds feature tests with less than the full complement of SDKs.