search

dapr / python-sdk

Dapr SDK for Python
Apache License 2.0
222 stars 125 forks source link

Add logs to Dapr Workflows #645

Closed shivamkm07 closed 8 months ago

shivamkm07 commented 9 months ago

Description

This PR does the following:

  1. Adds the support for adding logger in Dapr
  2. Adds logs in dapr-ext-workflow
  3. Passes the logger options to DTF-python to enable logging

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: #626

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

codecov[bot] commented 9 months ago

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (5c10b78) 86.43% compared to head (8eaba3c) 86.54%.

Files Patch % Lines
...orkflow/dapr/ext/workflow/dapr_workflow_context.py 66.66% 3 Missing :warning:
...pr-ext-workflow/dapr/ext/workflow/logger/logger.py 88.46% 3 Missing :warning:
...ext-workflow/dapr/ext/workflow/workflow_runtime.py 87.50% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #645 +/- ## ========================================== + Coverage 86.43% 86.54% +0.10% ========================================== Files 75 78 +3 Lines 3893 3953 +60 ========================================== + Hits 3365 3421 +56 - Misses 528 532 +4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

berndverst commented 9 months ago

My strong preference is for self contained PRs. So if you'd like to introduce a logger to be used for all Python SDKs, I would like to see that done in a single PR. That's why I am asking for the logger code to be moved to the workflow extension directory for now. As long as the code is self contained there I will approve :)

In the future we could still migrate that to be at the top level of the repo, but at that point I'd like the same PR to contain the switch to that logger throughout all SDKs.

shivamkm07 commented 8 months ago

My strong preference is for self contained PRs. So if you'd like to introduce a logger to be used for all Python SDKs, I would like to see that done in a single PR. That's why I am asking for the logger code to be moved to the workflow extension directory for now. As long as the code is self contained there I will approve :)

In the future we could still migrate that to be at the top level of the repo, but at that point I'd like the same PR to contain the switch to that logger throughout all SDKs.

Done. @berndverst Please review