Refactor/task builder #131

JonatanMartens · 2021-02-27T17:32:11Z

Move the task building process from ZeebeWorker.
This pr introduces a breaking change.

Changes

Add TaskConfig class, which is used internally
Remove TaskHandler class
Remove ZeebeTaskDecorator base class
- ZeebeWorker now inherits from ZeebeTaskRouter

API Updates

none

Enhancements (optional)

Move task building functionality out of the ZeebeWorker class.

Checklist

Unit tests
Documentation

…nfiguration

…ypes

…rker

This allows for parameters named args or kwargs that are not *args or **kwargs

JonatanMartens · 2021-02-27T17:34:09Z

Considering this is a breaking change, I think it is a good idea to include zeebe v1.0.0 support before releasing. See https://forum.zeebe.io/t/changelog-for-zeebe-client-lib-maintainers/2064

pyzeebe/task/task.py

kbakk

Approached this by going over the tests added/changed.

pyzeebe/task/task_config.py

pyzeebe/worker/task_router.py

tests/unit/worker/task_router_test.py

kbakk · 2021-03-05T19:06:02Z

tests/unit/task/task_builder_test.py

+    def test_decorator_variables_are_added(self, original_task_function: Callable, decorator: TaskDecorator,
+                                           task_config: TaskConfig, mocked_job_with_adapter: Job):
+        decorator_return_value = mocked_job_with_adapter
+        decorator_return_value.variables = {"x": 2}


Maybe create a helper function that returns {str(uuid4()): str(uuid4)}

pyzeebe/task/task_builder.py

tests/unit/task/task_builder_test.py

tests/unit/worker/worker_test.py

The dataclasses library is not supported in python 3.6

kbakk

Approached this review round from the code side (as well as verified the recent changes).

Looks good, some suggestions/changes needed. Impressive work, thanks for your commitment to this project!

pyzeebe/task/task_config.py

pyzeebe/task/task_builder.py

pyzeebe/task/task.py

pyzeebe/worker/task_router.py

…_raised

JonatanMartens · 2021-03-15T20:24:37Z

Hey @kbakk, I implemented most of your suggestions, some replies to things I view differently/didn't understand. Could you take another look?

kbakk · 2021-03-18T10:46:26Z

A couple of lint issues to address, otherwise good to merge I think:

JonatanMartens · 2021-03-20T14:40:21Z

Hey @kbakk, I decided that ZeebeTaskRouter.task should keep the same API as before. There is no benefit in forcing the user to create another object just to create a task. It also solves some of your comments and leads to some cleaner code. I would love it if you could take another look!

kbakk

Sorry for the delay here.

I think it's a good call to retain the task API.

Couple of comments, not important to address, so setting this to approved. Well done!

pyzeebe/worker/task_router.py

kbakk · 2021-03-26T17:18:40Z

pyzeebe/worker/task_router.py

-
-        def task_wrapper(fn: Callable):
-            task = task_builder.build_task(fn, config_with_decorators)
+        if single_value and not variable_name:


Can we move back this check to TaskConfig.__init__, and build the TaskConfig outside of the wrapper? It seems to belong there.

The task config is built here because we need the task_function to get variables_to_fetch

JonatanMartens added 22 commits February 20, 2021 14:51

Add task builder

ff459bd

Add single_value function to dict conversion to task_builder

bd11a09

Move task_builder from pyzeebe/worker to pyzeebe/task

7095d9e

Add type annotations to build_task

0f659fd

Add type annotations to build_task

dc57eff

Add log when failing to execute job

0ab0445

Refactor Task to be a dataclass containing the job_handler and the co…

89b1667

…nfiguration

Add pyzeebe/task/types

67b0a0a

Move TaskDecorator from pyzeebe/task/task_decorator to pyzeebe/task/t…

2c3f931

…ypes

Refactor task_handler to accept TaskConfig

d3f2aba

Rename TaskHandler to ZeebeTaskRouter

b8c3a94

Refactor ZeebeTaskRouter tests

59131d0

Refactor ZeebeWorker to use task_builder

3bb171d

Fix worker tests

f52ac70

Rename functions named _ to dummy_function

cb03ed4

Add type annotations to ZeebeWorker tests

1057465

Export TaskConfig, default_exception_handler

d920ed9

Remove redundant import from zeebe_worker.py

d2c4d04

Remove ZeebeDecoratorBase

48cb320

Refactor integration tests to use pytest fixtures and use new ZeebeWo…

d02ae2b

…rker

Use inspect to get function parameters

579055d

This allows for parameters named args or kwargs that are not *args or **kwargs

Import Callable from typing instead of collections

67710f1

JonatanMartens added breaking change enhancement New feature or request labels Feb 27, 2021

kbakk self-requested a review March 5, 2021 13:34

kbakk reviewed Mar 5, 2021

View reviewed changes

pyzeebe/task/task.py Outdated Show resolved Hide resolved

kbakk reviewed Mar 5, 2021

View reviewed changes

JonatanMartens mentioned this pull request Mar 6, 2021

Pyzeebe 3.0.0 #136

Closed

Remove usage of dataclasses

409d114

The dataclasses library is not supported in python 3.6

JonatanMartens added 4 commits March 6, 2021 12:57

Refactor build_job_handler tests to fit arrange, act, assert structure

d861280

Use pytest parameterize to simplify TestGetFunctionParameters

1da1d0e

Improve default_exception_handler test name

2a2eaf7

Add happy path to task_config tests

f746baa

kbakk requested changes Mar 8, 2021

View reviewed changes

JonatanMartens added 7 commits March 14, 2021 22:08

Add docstrings to before and after methods

8d7547b

Rename timeout field in TaskConfig to timeout_ms

f63f1d0

Clean get_parameters_from_function function inner names

62c6002

Add repr to Task and TaskConfig objects

2b8ddb2

Rename wrapper to task_wrapper

bd568e2

Rename fixture from task_handler_mock to job_handler_spy

3483cf1

Call route._is_task_duplicate in test_no_duplicate_task_type_error_is…

83c4ce6

…_raised

JonatanMartens changed the base branch from development to pre-release/3.0.0 March 15, 2021 20:26

kbakk mentioned this pull request Mar 18, 2021

Raised exception in decorator should prevent further processing #149

Open

Receive regular params instead of task config

3c57237

JonatanMartens removed the breaking change label Mar 20, 2021

JonatanMartens and others added 5 commits March 20, 2021 16:54

Add test case for no variables added to result

876d819

Run tests on every push

6d9e85e

Merge branch 'pre-release/3.0.0' into refactor/task-builder

c29addb

Fix integration tests to work without task config

34e7f26

Update ZeebeWorker documentation

4a8ba92

JonatanMartens requested a review from kbakk March 26, 2021 14:19

kbakk approved these changes Mar 26, 2021

View reviewed changes

JonatanMartens added 2 commits March 27, 2021 15:30

Remove docstring whitespace in ZeebeTaskRouter

22042f4

Move variable name check back to TaskConfig

1621369

JonatanMartens merged commit 73feb0b into pre-release/3.0.0 Mar 27, 2021

JonatanMartens deleted the refactor/task-builder branch March 27, 2021 12:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor/task builder #131

Refactor/task builder #131

JonatanMartens commented Feb 27, 2021 •

edited

Loading

JonatanMartens commented Feb 27, 2021

kbakk left a comment

kbakk Mar 5, 2021

kbakk left a comment

JonatanMartens commented Mar 15, 2021

kbakk commented Mar 18, 2021

JonatanMartens commented Mar 20, 2021

kbakk left a comment

kbakk Mar 26, 2021

JonatanMartens Mar 27, 2021

Refactor/task builder #131

Refactor/task builder #131

Conversation

JonatanMartens commented Feb 27, 2021 • edited Loading

Changes

API Updates

Enhancements (optional)

Checklist

JonatanMartens commented Feb 27, 2021

kbakk left a comment

Choose a reason for hiding this comment

kbakk Mar 5, 2021

Choose a reason for hiding this comment

kbakk left a comment

Choose a reason for hiding this comment

JonatanMartens commented Mar 15, 2021

kbakk commented Mar 18, 2021

JonatanMartens commented Mar 20, 2021

kbakk left a comment

Choose a reason for hiding this comment

kbakk Mar 26, 2021

Choose a reason for hiding this comment

JonatanMartens Mar 27, 2021

Choose a reason for hiding this comment

JonatanMartens commented Feb 27, 2021 •

edited

Loading