Skip to content

Python: Improve handling the Dapr process context #10725

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

Merged
merged 7 commits into from
Mar 5, 2025

Conversation

moonbox3
Copy link
Contributor

@moonbox3 moonbox3 commented Feb 28, 2025

Motivation and Context

There is a bug in the current Dapr runtime code that prohibits trying to get the Dapr process context out from a process once it's finished. This is related to incorrect handling of serializing/deserializing the Dapr process context. Additionally, in a state that handles KernelProcessStepState, due to Dapr serialization constraints, the step step injected to a step is not the actual object. We use Pydantic's model_validate to get the actual step object, but and we need to make sure we re-assign that object back to the underlying step state so the objects don't diverge (model_validate creates a new object).

Description

Improve the handling for the Dapr process context so we're properly serializing data through the Dapr process.

Contribution Checklist

@moonbox3 moonbox3 requested a review from a team as a code owner February 28, 2025 11:00
@moonbox3 moonbox3 added python Pull requests for the Python Semantic Kernel processes labels Feb 28, 2025
@moonbox3 moonbox3 marked this pull request as draft February 28, 2025 11:06
@markwallace-microsoft
Copy link
Member

markwallace-microsoft commented Feb 28, 2025

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
semantic_kernel/processes/dapr_runtime
   dapr_kernel_process_context.py35294%25, 27
   dapr_step_info.py32294%31, 40
semantic_kernel/processes/dapr_runtime/actors
   process_actor.py24511453%72–76, 81, 84, 90, 93, 97, 102, 105, 121–124, 129, 142, 160–164, 169, 179, 184, 193–204, 208–210, 218–234, 239, 255–316, 321, 332, 346–354, 357–364, 368–396, 427–440
   step_actor.py26313349%81–85, 90, 93, 97–98, 110–112, 156–159, 173–175, 183, 187–188, 214, 222–223, 239–246, 253–254, 262–360, 364, 368–371, 375–391, 395–419, 423–441, 445–448, 452, 456–459
semantic_kernel/processes/local_runtime
   local_process.py1385362%66, 103, 113, 131–142, 176–203, 207–212, 216, 220–226, 230–241, 245–246
TOTAL19594255487% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
3207 5 💤 0 ❌ 0 🔥 1m 32s ⏱️

@moonbox3 moonbox3 marked this pull request as ready for review March 5, 2025 00:50
@moonbox3 moonbox3 enabled auto-merge March 5, 2025 10:45
@moonbox3 moonbox3 added this pull request to the merge queue Mar 5, 2025
Merged via the queue into microsoft:main with commit fb70825 Mar 5, 2025
28 checks passed
@github-project-automation github-project-automation bot moved this to Sprint: Done in Semantic Kernel Mar 5, 2025
musale pushed a commit to musale/semantic-kernel that referenced this pull request Mar 10, 2025
### Motivation and Context

There is a bug in the current Dapr runtime code that prohibits trying to
get the Dapr process context out from a process once it's finished. This
is related to incorrect handling of serializing/deserializing the Dapr
process context. Additionally, in a state that handles
`KernelProcessStepState`, due to Dapr serialization constraints, the
step step injected to a step is not the actual object. We use Pydantic's
`model_validate` to get the actual step object, but and we need to make
sure we re-assign that object back to the underlying step state so the
objects don't diverge (`model_validate` creates a new object).

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

### Description

Improve the handling for the Dapr process context so we're properly
serializing data through the Dapr process.
- Fixes the issue brought up in the discussion: microsoft#10234

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [X] The code builds clean without any errors or warnings
- [X] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [X] All unit tests pass, and I have added new tests where possible
- [X] I didn't break anyone 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
processes python Pull requests for the Python Semantic Kernel
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants