-
Notifications
You must be signed in to change notification settings - Fork 128
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
workflows: update durabletask dependency #757
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Fabian Martinez <[email protected]>
Signed-off-by: Fabian Martinez <[email protected]>
Signed-off-by: Fabian Martinez <[email protected]>
Signed-off-by: Fabian Martinez <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments. Please run tox -e ruff
and tox -e flake8
to fix linter errors.
We'll also need to update the grpc library to >= 1.67.0, for compatibility with durabletask.
And finally, we'll need to update the tests in test_dapr_workflow_context.py
to cover set-custom_status
. Something like
class FakeOrchestrationContext:
def __init__(self):
self.instance_id = mock_instance_id
self.custom_status = None. # <- adding this
def set_custom_status(self, custom_status):
self.custom_status = custom_status
...
class DaprWorkflowContextTest(unittest.TestCase):
...
def test_workflow_context_functions(self):
...
dapr_wf_ctx.set_custom_status(mock_status)
assert fakeContext.custom_status == mock_status
ext/dapr-ext-workflow/dapr/ext/workflow/dapr_workflow_client.py
Outdated
Show resolved
Hide resolved
ext/dapr-ext-workflow/dapr/ext/workflow/dapr_workflow_client.py
Outdated
Show resolved
Hide resolved
ext/dapr-ext-workflow/dapr/ext/workflow/dapr_workflow_client.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Elena Kolevska <[email protected]> Signed-off-by: Fabian Martinez <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]> Signed-off-by: Fabian Martinez <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]> Signed-off-by: Fabian Martinez <[email protected]>
Co-authored-by: Elena Kolevska <[email protected]> Signed-off-by: Fabian Martinez <[email protected]>
Signed-off-by: Fabian Martinez <[email protected]>
@@ -26,8 +26,8 @@ include_package_data = True | |||
zip_safe = False | |||
install_requires = | |||
protobuf >= 4.22 | |||
grpcio >= 1.37.0 | |||
grpcio-status>=1.37.0 | |||
grpcio >= 1.67.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@berndverst any objections on bumping the grpc library version?
This has been updated in the durable task fork and it's a dependency here.
Signed-off-by: Elena Kolevska <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #757 +/- ##
==========================================
- Coverage 86.63% 86.07% -0.56%
==========================================
Files 84 87 +3
Lines 4473 4804 +331
==========================================
+ Hits 3875 4135 +260
- Misses 598 669 +71 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, pending @berndverst 's approval on the grpcio version bump
Description
Update durabletask dependency to use new dapr fork
Also implements support for purge API #711
Implements support for reuse id policy and set custom status from #739
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #[issue number]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: