dapr / python-sdk

Dapr SDK for Python
Apache License 2.0
232 stars 127 forks source link

only serialize failure details if they exist #707

Closed akshaya-a closed 5 months ago

akshaya-a commented 6 months ago

Description

Please explain the changes you've made

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

Checklist

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

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 86.15%. Comparing base (fc0e9d1) to head (debd571). Report is 26 commits behind head on main.

Files Patch % Lines
...r-ext-workflow/dapr/ext/workflow/workflow_state.py 0.00% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #707 +/- ## ========================================== - Coverage 86.37% 86.15% -0.22% ========================================== Files 79 79 Lines 4094 4169 +75 ========================================== + Hits 3536 3592 +56 - Misses 558 577 +19 ```

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

elena-kolevska commented 6 months ago

Hey @akshaya-a , thanks for your PR. Could you please sign your commit? Here's how to do it:

Rebase the branch

If you have a local git environment and meet the criteria below, one option is to rebase the branch and add your Signed-off-by lines in the new commits. Please note that if others have already begun work based upon the commits in this branch, this solution will rewrite history and may cause serious issues for collaborators (described in the git documentation under "The Perils of Rebasing").

You should only do this if:

You are the only author of the commits in this branch
You are absolutely certain nobody else is doing any work based upon this branch
There are no empty commits in the branch (for example, a DCO Remediation Commit which was added using --allow-empty)

To add your Signed-off-by line to every commit in this branch:

Ensure you have a local copy of your branch by [checking out the pull request locally via command line](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/checking-out-pull-requests-locally).
In your local branch, run: git rebase HEAD~1 --signoff
Force push your changes to overwrite the branch: git push --force-with-lease origin ak/guard-workflow-state-failure-details
elena-kolevska commented 6 months ago

I was able to reproduce this locally and test the fix. Looks good. I would love an added test for it, though.

berndverst commented 6 months ago

@akshaya-a please do the following or this cannot be merged.

To add your Signed-off-by line to every commit in this branch:

Ensure you have a local copy of your branch by checking out the pull request locally via command line. In your local branch, run: git rebase HEAD~1 --signoff Force push your changes to overwrite the branch: git push --force-with-lease origin ak/guard-workflow-state-failure-details

akshaya-a commented 6 months ago

@elena-kolevska @berndverst sorry for the delay, was focused on some stuff for //build

yeah the only reason I didn't bother signing yet was I assumed we'd want a test covering the failure but in my project I worked around it by just reaching in to the .__obj for debugging purposes, reducing the immediate priority

i can probably get to this next week?

akshaya-a commented 6 months ago

ah didn't realize i already had a codespace for this repo, no need to be lazy - signed in case you want to merge, but happy to add a test later

marcduiker commented 3 months ago

@holopin-bot @akshaya-a Thank you!

holopin-bot[bot] commented 3 months ago

Congratulations @akshaya-a, the maintainer of this repository has issued you a badge! Here it is: https://holopin.io/claim/clzvb3ac070950cjy3pllbfva

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account. Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!