Skip to content

Commit 73ad6e8

Browse files
MMoshtaghimoonbox3
andauthored
Python: Fix: Implement __deepcopy__ on KernelFunction to handle non-serializable OTEL metrics (#12803)
### Description This PR fixes TypeError: cannot pickle '_thread.RLock' object which occurs when kernel.clone() is called while OpenTelemetry metrics are enabled. - Fixes: #12802 ### Motivation and Context When using features like `HandoffOrchestration`, the agent's `Kernel` is cloned. This process uses `copy.deepcopy`, which fails on the `KernelFunction`'s OpenTelemetry `Histogram` fields (`invocation_duration_histogram` and `streaming_duration_histogram`). These histogram objects contain thread locks, making them non-serializable. Previous solutions, like the one in **PR #9292**, used `Field(exclude=True, default_factory=...)`. While that was a necessary step, it was insufficient because `deepcopy` does not respect the `exclude` flag in the same way that Pydantic's serialization does. This bug blocks the use of certain agentic features in any application that has metrics-based observability enabled. ### Description of the change This solution introduces a custom __deepcopy__ method to the KernelFunction Pydantic model. This method correctly handles the deep-copying process for KernelFunction instances: 1. It intercepts the deepcopy call for KernelFunction objects. 2. It uses self.model_copy(deep=False) to create a new instance of the Pydantic model. Using deep=False allows the default_factory for the histogram fields to create new, clean instances on the copied object, while other fields are shallow-copied. 3. It then manually deep-copies all other attributes from the original object to the new one, ensuring that any other mutable objects (like metadata`) are correctly duplicated. The histogram fields are explicitly skipped in this step, as they have already been recreated by `model_copy. 4. Standard deepcopy memoization is used to prevent infinite recursion in the case of cyclic object graphs. This approach ensures that the non-serializable fields are re-initialized instead of copied, resolving the TypeError while maintaining the integrity of the cloned object. ### How has this been tested? The fix was validated using the reproduction steps outlined in the associated issue. Running an agent orchestration that triggers kernel.clone() with OpenTelemetry metrics enabled now completes successfully without any errors. ### 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 😄 --------- Co-authored-by: Evan Mattson <35585003+moonbox3@users.noreply.github.com>
1 parent a6b5f34 commit 73ad6e8

File tree

7 files changed

+1313
-1069
lines changed

7 files changed

+1313
-1069
lines changed

python/semantic_kernel/connectors/azure_ai_search.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,18 @@ def _parse_attribute_chain(attr_node: ast.Attribute) -> str:
675675
case ast.Name():
676676
raise NotImplementedError("Constants are not supported, make sure to use a value or a attribute.")
677677
case ast.Constant():
678-
return str(node.value) if isinstance(node.value, float | int) else f"'{node.value}'"
678+
value = node.value
679+
if isinstance(value, str):
680+
return "'" + value.replace("'", "''") + "'"
681+
if isinstance(value, bytes):
682+
return "'" + value.decode("utf-8").replace("'", "''") + "'"
683+
if isinstance(value, bool):
684+
return str(value).lower()
685+
if value is None:
686+
return "null"
687+
if isinstance(value, (int, float)):
688+
return str(value)
689+
raise VectorStoreOperationException(f"Unsupported constant type: {type(value)}")
679690
raise NotImplementedError(f"Unsupported AST node: {type(node)}")
680691

681692
@override

python/semantic_kernel/connectors/chroma.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,14 @@ def _lambda_parser(self, node: ast.AST) -> dict[str, Any] | str | int | float |
422422
)
423423
return node.id
424424
case ast.Constant():
425-
return node.value
425+
value = node.value
426+
if isinstance(value, str):
427+
return value.replace("'", "''")
428+
if isinstance(value, bytes):
429+
return value.decode("utf-8").replace("'", "''")
430+
if isinstance(value, (int, float, bool)) or value is None:
431+
return value
432+
raise VectorStoreOperationException(f"Unsupported constant type: {type(value)}")
426433
raise NotImplementedError(f"Unsupported AST node: {type(node)}")
427434

428435

python/semantic_kernel/connectors/sql_server.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -637,9 +637,11 @@ def parse(node: ast.AST) -> str:
637637
)
638638
return f"[{node.id}]"
639639
case ast.Constant():
640-
# Always use parameterization for constants
641-
command.add_parameter(node.value)
642-
return "?"
640+
value = node.value
641+
if isinstance(value, (str, int, float, bool, bytes)) or value is None:
642+
command.add_parameter(str(value))
643+
return "?"
644+
raise VectorStoreOperationException(f"Unsupported constant type: {type(value)}")
643645
case ast.List():
644646
# For IN/NOT IN lists, parameterize each element
645647
placeholders = []

python/semantic_kernel/connectors/weaviate.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,14 @@ def _lambda_parser(self, node: ast.AST) -> "_Filters | FilterValues":
469469
)
470470
return node.id
471471
case ast.Constant():
472-
return node.value
472+
value = node.value
473+
if isinstance(value, str):
474+
return value.replace("'", "''")
475+
if isinstance(value, bytes):
476+
return value.decode("utf-8").replace("'", "''")
477+
if isinstance(value, (int, float, bool)) or value is None:
478+
return value
479+
raise VectorStoreOperationException(f"Unsupported constant type: {type(value)}")
473480
raise NotImplementedError(f"Unsupported AST node: {type(node)}")
474481

475482
async def _inner_vectorized_search(

python/semantic_kernel/functions/kernel_function.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,25 @@ class KernelFunction(KernelBaseModel):
100100
default_factory=_create_function_streaming_duration_histogram, exclude=True
101101
)
102102

103+
def __deepcopy__(self, memo: dict[int, Any] | None = None) -> "KernelFunction":
104+
"""Create a deep copy of the kernel function, recreating uncopyable fields."""
105+
if memo is None:
106+
memo = {}
107+
if id(self) in memo:
108+
return memo[id(self)]
109+
110+
# Use model_copy to create a shallow copy of the pydantic model
111+
# this is the recommended way to copy pydantic models
112+
new_obj = self.model_copy(deep=False)
113+
memo[id(self)] = new_obj
114+
115+
# now deepcopy the fields that are not the histograms
116+
for key, value in self.__dict__.items():
117+
if key not in ("invocation_duration_histogram", "streaming_duration_histogram"):
118+
setattr(new_obj, key, deepcopy(value, memo))
119+
120+
return new_obj
121+
103122
@classmethod
104123
def from_prompt(
105124
cls,

python/tests/unit/functions/test_kernel_function_from_prompt.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import os
44
import tempfile
5+
from copy import deepcopy
56
from unittest.mock import patch
67

78
import pytest
@@ -626,3 +627,22 @@ def test_from_directory_backward_compatibility():
626627
function = KernelFunctionFromPrompt.from_directory(temp_dir)
627628
assert function.description == "Basic function"
628629
assert function.prompt_template.prompt_template_config.template == "Basic ASCII content: {{$input}}"
630+
631+
632+
def test_kernel_function_from_prompt_deepcopy():
633+
"""Test deepcopying a KernelFunctionFromPrompt."""
634+
function = KernelFunctionFromPrompt(
635+
function_name="test_function",
636+
plugin_name="test_plugin",
637+
prompt="Hello, world!",
638+
description="A test function.",
639+
)
640+
copied_function = deepcopy(function)
641+
assert copied_function is not function
642+
assert copied_function.name == function.name
643+
assert copied_function.plugin_name == function.plugin_name
644+
assert copied_function.description == function.description
645+
assert copied_function.prompt_template.prompt_template_config.template == (
646+
function.prompt_template.prompt_template_config.template
647+
)
648+
assert copied_function.prompt_template is not function.prompt_template

0 commit comments

Comments
 (0)