Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

FIXED

- Dataclasses with `init=False` fields now reconstruct as their declared type
instead of falling back to a raw dictionary. Derived fields are initialized by
the dataclass constructor rather than passed as unsupported keyword arguments.
- Timer callbacks no longer schedule additional long-timer chunks, retry
activities or sub-orchestrations, or resume orchestrator code after completion,
failure, termination, or continue-as-new. Work scheduled before the terminal
Expand Down
4 changes: 3 additions & 1 deletion durabletask/serialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,9 @@ def _build_dataclass(cls: Any, data: dict[str, Any],
globalns = _type_namespace(cls)
kwargs: dict[str, Any] = {}
for field in dataclasses.fields(cls):
if field.name not in data:
# Derived fields are serialized, but are initialized by the dataclass
# itself (for example in __post_init__), not by constructor arguments.
if not field.init or field.name not in data:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please preserve serialized init=False values when the actual constructor accepts them. field.init describes the generated initializer, not the signature of a handwritten initializer. This currently causes silent data loss for existing supported dataclasses:

from dataclasses import dataclass, field
from durabletask.serialization import JsonDataConverter

@dataclass
class CounterState:
    name: str
    counter: int = field(init=False, default=0)

    def __init__(self, name: str, counter: int = 0):
        self.name = name
        self.counter = counter

converter = JsonDataConverter()
encoded = converter.serialize(CounterState("persisted", 42))
restored = converter.deserialize(encoded, CounterState)
# Before this PR: restored.counter == 42
# With this PR:  restored.counter == 0

This also reproduces through StateShim.get_state(CounterState), and writing the reconstructed state persists the reset value.

Recommended change: Omit a non-init field only when the constructor cannot accept it as a keyword argument.

  • Keep the existing handling of init=True fields unchanged.
  • For a serialized init=False field, inspect the actual initializer for an explicit keyword-capable parameter with that name or **kwargs. Pass and recursively coerce the value when accepted; otherwise omit it so generated constructors can initialize derived fields.
  • Use a bounded signature cache keyed by the initializer callable, and avoid inspection for dataclasses without non-init fields. If inspection is unavailable, retain the legacy behavior of passing the field rather than silently dropping data.
  • Do not invoke the constructor and retry on TypeError: the exception may come from inside user code, and retrying may run side effects twice. Do not blindly restore attributes after construction either, since that could overwrite intentionally computed values or bypass frozen-dataclass semantics.

Please add regressions for a handwritten constructor with an explicit non-init parameter and one accepting **kwargs, including preservation of 42 through typed entity-state read/write, while retaining the generated/frozen/nested derived-field cases. This is a targeted compatibility fix; arbitrary non-init state with a generated constructor still needs explicit reconstruction semantics, as noted in the review summary.

continue
# ``get_type_hints`` on Python 3.10 does not deep-resolve forward
# references nested inside container args (e.g. the ``"TreeNode"`` in
Expand Down
38 changes: 37 additions & 1 deletion tests/durabletask/test_data_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import json
import logging
from dataclasses import dataclass
from dataclasses import dataclass, field
from typing import Any

from durabletask.serialization import (
Expand All @@ -21,6 +21,42 @@ class Order:
quantity: int


@dataclass(frozen=True)
class PricedOrder:
quantity: int
unit_price: int
total: int = field(init=False)

def __post_init__(self):
object.__setattr__(self, "total", self.quantity * self.unit_price)


@dataclass
class Shipment:
order: PricedOrder


def test_round_trip_dataclass_with_derived_field():
converter = JsonDataConverter()
order = PricedOrder(3, 10)
encoded = converter.serialize(order)
assert json.loads(encoded) == {"quantity": 3, "unit_price": 10, "total": 30}
assert converter.deserialize(encoded, PricedOrder) == order


def test_coerce_dataclass_recomputes_derived_field():
converter = JsonDataConverter()
result = converter.coerce({"quantity": 3, "unit_price": 10, "total": 999}, PricedOrder)
assert result == PricedOrder(3, 10)
assert result.total == 30


def test_round_trip_nested_dataclass_with_derived_field():
converter = JsonDataConverter()
shipment = Shipment(PricedOrder(3, 10))
assert converter.deserialize(converter.serialize(shipment), Shipment) == shipment


# ----- JsonDataConverter -----


Expand Down