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

Add tests of asyncio.Lock and asyncio.Semaphore usage #567

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dandavison
Copy link
Contributor

Fixes #554

Add tests confirming that asyncio.Lock and asyncio.Semaphore can be used in workflow code to control concurrency of coroutines (tasks spawned by the main coroutine, and update handlers).

The fact that these tests pass largely follows from the fact that our event loop implementation has functioning implementations of call_later() call_soon(), so these tests could alternatively be moved into the features repo, as part of a feature test confirming that all languages have mutex/semaphore primitives that can be used for this sort of workflow concurrency control.

@dandavison dandavison force-pushed the sdk-2358-workflow-mutex-and-semaphore branch from 87fc797 to 390ee94 Compare July 1, 2024 14:12
@dandavison dandavison marked this pull request as ready for review July 1, 2024 15:35
@dandavison dandavison requested a review from a team as a code owner July 1, 2024 15:35
@dandavison dandavison force-pushed the sdk-2358-workflow-mutex-and-semaphore branch from 390ee94 to 6eb176a Compare July 1, 2024 15:40
@dandavison dandavison force-pushed the sdk-2358-workflow-mutex-and-semaphore branch from 94d0d38 to 2e68dad Compare July 9, 2024 18:00
Comment on lines 425 to 426
if seen_completion:
self._warn_if_unfinished_handlers()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this removed intentionally?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry I pushed a temp commit for the cancellation sample I sent you earlier, to force push over later. Force-pushed now.

@dandavison dandavison force-pushed the sdk-2358-workflow-mutex-and-semaphore branch from 2a3a9c3 to 6eb176a Compare July 9, 2024 21:52
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM, granted I apply very little scrutiny to test code

@dandavison dandavison force-pushed the sdk-2358-workflow-mutex-and-semaphore branch from 6eb176a to 0549ac8 Compare July 9, 2024 22:19
@dandavison dandavison force-pushed the sdk-2358-workflow-mutex-and-semaphore branch from 0549ac8 to a524593 Compare July 11, 2024 08:04
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

Successfully merging this pull request may close these issues.

Workflow-friendly concurrency control
2 participants