Skip to content

Conversation

@Gustavo-Hagenbeck
Copy link
Contributor

@Gustavo-Hagenbeck Gustavo-Hagenbeck commented Dec 4, 2025

Summary

Fixes the default serde parameter in ServerInvocationContext.promise() to use DefaultSerde() instead of JsonSerde(), aligning with the interface definition in WorkflowContext.

Changes

  • Changed default parameter from JsonSerde() to DefaultSerde() in promise() method
  • Removed unused JsonSerde import
  • Add unit test that verifies if the default for promise() is DefaultSerde

Motivation

The default serde is supposed to be DefaultSerde, as defined in WorkflowContext. This bug forces users to explicitly declare serde=DefaultSerde(), even though it is already documented as the default in WorkflowContext.
Slack thread that originated this PR

Testing

  • ✅ All linting checks pass
  • ✅ Type checking passes (pyright & mypy)
  • ✅ All 10 unit tests pass

Linting

  $ uv run ruff format
  53 files left unchanged

  $ uv run ruff check
  All checks passed!

Type Checking

  $ PYRIGHT_PYTHON_IGNORE_WARNINGS=1 uv run pyright python/
  0 errors, 0 warnings, 0 informations

  $ uv run mypy --check-untyped-defs --ignore-missing-imports python/
  Success: no issues found in 24 source files

Unit Tests

  $ uv run pytest tests/*.py -v
  ============================= test session starts ==============================
  platform darwin -- Python 3.12.0, pytest-8.4.2, pluggy-1.6.0
  rootdir: /Users/gustavogomes/projects/sdk-python
  configfile: pyproject.toml
  plugins: anyio-4.11.0
  collecting ... collected 11 items

  tests/ext.py::test_greeter PASSED                                        [  9%]
  tests/ext.py::test_greeter_with_cm PASSED                                [ 18%]
  tests/harness.py::test_greeter PASSED                                    [ 27%]
  tests/harness.py::test_counter PASSED                                    [ 36%]
  tests/harness.py::test_idempotency_key PASSED                            [ 45%]
  tests/harness.py::test_workflow PASSED                                   [ 54%]
  tests/harness.py::test_send PASSED                                       [ 63%]
  tests/serde.py::test_bytes_serde PASSED                                  [ 72%]
  tests/servercontext.py::test_sanity PASSED                               [ 81%]
  tests/servercontext.py::test_wrapped_terminal_exception PASSED           [ 90%]
  tests/servercontext.py::test_promise_default_serde PASSED                [100%]

  ============================= 11 passed in 16.04s ==============================

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@Gustavo-Hagenbeck
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

The default serde is supposed to be DefaultSerde, as defined in WorkflowContext. This bug forces users to explicitly declare serde=DefaultSerde(), even though it is already documented as the default in WorkflowContext.
@Gustavo-Hagenbeck Gustavo-Hagenbeck force-pushed the fix-promise-default-serde branch from c789a77 to b1a64bb Compare December 4, 2025 12:51
@Gustavo-Hagenbeck Gustavo-Hagenbeck marked this pull request as ready for review December 4, 2025 12:53
Copy link
Contributor

@igalshilman igalshilman left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the PR!

@igalshilman igalshilman merged commit 527728c into restatedev:main Dec 4, 2025
6 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants