Feature/async #172
Feature/async #172
Conversation
JobPoller is a new class in preperation for pyzeebe's move to asyncio.
This commit adds support for asyncio in both workers and clients. It also adds a new class called JobExecutor, thus simplifying the ZeebeWorker class.
This achieves two things: 1. Allows setup without unexpected connections to zeebe (side effects) in a constructor 2. Solves an issue where if creating a worker and then calling worker.work() later, grpc.aio would use a different event loop
The Asyncmock package adds the AsynMock class to this package, not unittest.mock.
JonatanMartens
force-pushed
the
feature/async-worker
branch
from
d21c897
to
2020439
Compare
Pull Request Test Coverage Report for Build 937025768
💛 - Coveralls |
|
Making the client async only introduces some limitations on where the client can be used. The worker would typically be run in a dedicated worker process (I'd imagine), however the client may be used in a synchronous function (e.g. a Flask route). I think we should offer a synchronous client in addition to an async client for this reason. The synchronous client can be a wrapper around the async client. |
| @@ -0,0 +1,10 @@ | |||
| from typing import Awaitable, Callable, Dict, TypeVar, Union | |||
There was a problem hiding this comment.
Should this module be prefixed with _ to indicate it's internal?
There was a problem hiding this comment.
Truly amazing work! Well done!
Have read through most of the changes, but there's a lot I don't fully understand here, and simply a lot of changes/additions, so I won't be able to review everything. I'll dive some more into the parts I'm more familiar with, and will add comments on them as well.
I think releasing this as a beta, and letting people test this, could be a good idea (maybe update the README in master to inform that there's effort going on, and we would like people to review or try it out).
|
A comment on the tests - they're blazing fast now! ✨🚀✨ 👏 |
There was a problem hiding this comment.
Some more input, after having tested.
I notice that pyzeebe/credentials/oauth_credentials.py is still using the synchronous requests module.
I came across this https://docs.authlib.org/en/latest/client/httpx.html#httpx-oauth-2-0 which could help solve that hurdle.
An issue I managed to consistently reproduce was the following:
- Create a basic worker that only prints the input variable
- Create a simple BPMN with 1 task, deploy 33 jobs on the broker - I used this oneliner:
for i in {1..33} ; do echo "Submitting $i..." && zbctl-1.0.0 --insecure create instance "Process_1n8eewe" --variables "{\"num\": $i}" ; done - Run the worker (important: it has to be started after the jobs have been deployed)
For me, the worker processes the 32 first jobs, then hangs.
I'm suspecting that it's getting RESOURCE_EXCHAUSTED but I'm not able to verify
I prefer to keep one interface. Adding a In practice, this class would just wrap all of |
Logger.warn is considered deprecated
Good idea! |
Co-authored-by: Kristoffer Bakkejord <[email protected]>
It's a second use case (use in synchronous context), so I think it may warrant that.
I think that if several users are to perform certain types of setup in a general way, it should be part of the library. Else they will have to reinvent the wheel, adding tests, possibly distributring it as a package (if it's used in different applications). According to the Python 2020 developer survey, 89 % used a web framework without async support (Flask and Django - maybe Django was introducing async around then, but I assume most users didn't start using that feature right away). From this, I'd say running synchronously is the primary way this will be used. Also, 100 % of the current use of Another take on this is to keep the synchronous parts of the client that exists today. |
…ub/pyzeebe into feature/async-worker
Because SyncZeebeClient overrides the normal ZeebeClient mypy expects SyncZeebeClient to also be async.
| async def test_raises_on_invalid_process(self, zeebe_adapter: ZeebeProcessAdapter): | ||
| with patch("builtins.open") as mock_open: | ||
| mock_open.return_value = BytesIO() | ||
| open_mock: MagicMock |
There was a problem hiding this comment.
I prefer using the tmp_path fixture, when validating file reads/writes.
https://docs.pytest.org/en/6.2.x/tmpdir.html#the-tmp-path-fixture
Full asyncio support in pyzeebe.
Changes
ZeebeClient's methods are asyncZeebeWorker.workis now asyncAPI Updates
New Features
Add support for asyncio tasks.
Deprecations
ZeebeWorker.workis now asyncZeebeClient's methods are now asyncgrpc_internalsnow usesgrpc.aioinstead of regulargrpcEnhancements
create_connection_uriandcreate_channelfromZeebeAdapterconnecttoZeebeAdapterto improve testability and solve agrpc.aioloop issue (see commit message)Checklist
References
Implements #144, #143