Skip to content

Preserve payloads in v1 status query shims - #236

Merged
andystaples merged 3 commits into
mainfrom
andystaples-address-issue-231
Jul 29, 2026
Merged

Preserve payloads in v1 status query shims#236
andystaples merged 3 commits into
mainfrom
andystaples-address-issue-231

Conversation

@andystaples

Copy link
Copy Markdown
Contributor

Summary

  • fetch payloads in the deprecated v1 get_status(), get_status_all(), and get_status_by() shims
  • apply show_input locally so disabling input does not discard output, custom status, or failure details
  • preserve native durabletask payload-fetch defaults and add running, completed, failed, and collection regression coverage

Testing

  • python -m pytest tests\azure-functions-durable -m "not dts and not azurite and not functions_e2e" --quiet
  • python -m flake8 azure-functions-durable
  • python -m flake8 tests\azure-functions-durable
  • python -m pyright -p azure-functions-durable\pyrightconfig.json

Fixes #231

Fetch orchestration payloads in the deprecated v1 status query shims while applying show_input locally. Preserve native durabletask query defaults and cover running, completed, failed, and collection status responses.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: ff9faf2d-e79a-4309-8a67-5d455e4dd7bd
Copilot AI review requested due to automatic review settings July 29, 2026 18:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the deprecated v1-compatible status query shims in azure-functions-durable so they fetch orchestration payloads needed to reproduce v1 responses, while applying show_input locally to suppress only the input field (without discarding output, custom status, or failure details).

Changes:

  • Updated get_status(), get_status_all(), and get_status_by() shims to ensure payloads are fetched (without changing native durabletask defaults).
  • Extended DurableOrchestrationStatus to support suppressing input via an internal include_input flag while preserving other payload fields.
  • Added regression coverage for running/completed/failed status projections and updated the package changelog.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
azure-functions-durable/azure/durable_functions/client.py Forces payload fetch in v1 status shim APIs and projects show_input in the wrapper.
azure-functions-durable/azure/durable_functions/internal/compat/durable_orchestration_status.py Adds include_input support to hide input in v1 JSON/status surface without affecting other payload fields.
tests/azure-functions-durable/test_client_compat.py Adds/updates tests to validate payload-fetch and v1 projection behavior across runtime statuses and collection shims.
azure-functions-durable/CHANGELOG.md Documents the user-visible fix under FIXED.

Comment thread azure-functions-durable/azure/durable_functions/client.py Outdated
Copilot AI review requested due to automatic review settings July 29, 2026 19:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

tests/azure-functions-durable/test_client_compat.py:321

  • test_get_status_by_maps_statuses inspects mock.await_args but doesn’t assert the shim awaited get_all_orchestration_states() exactly once. Adding a call-count assertion makes the delegation contract explicit and prevents false positives if the shim calls the API multiple times.
        query = mock.await_args.args[0]
        assert query.runtime_status == [OrchestrationStatus.RUNNING]
        assert query.fetch_inputs_and_outputs is True

tests/azure-functions-durable/test_client_compat.py:202

  • test_get_status_all_delegates no longer asserts that get_all_orchestration_states() was awaited exactly once. Without an explicit call-count assertion, the test could pass even if the shim makes multiple calls (or retries) and only the last call has the expected query.

This issue also appears on line 319 of the same file.

        query = mock.await_args.args[0]
        assert query.fetch_inputs_and_outputs is True

tests/azure-functions-durable/test_client_compat.py:589

  • The regression test for failed instances validates to_json()["output"] == "boom", but it doesn’t validate the compatibility object’s status.output property. If v1 callers read status.output directly (common), they could still miss the failure message even though to_json() includes it. Consider asserting status.output == "boom" here and updating DurableOrchestrationStatus.output to fall back to failure_details.message when serialized_output is None.
        status_json = status.to_json()
        assert status_json["runtimeStatus"] == "Failed"
        assert status_json["output"] == "boom"
        assert status_json["customStatus"] == "cs"
        assert "input" not in status_json

@berndverst berndverst left a comment

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.

One compatibility gap remains: failed statuses expose the failure text only through to_json(), not through the documented .output property.

Comment thread tests/azure-functions-durable/test_client_compat.py
Expose failed orchestration messages through the compatibility output property, reuse that projection in JSON output, clarify the shim documentation, and strengthen delegation assertions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: ff9faf2d-e79a-4309-8a67-5d455e4dd7bd
Copilot AI review requested due to automatic review settings July 29, 2026 19:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

tests/azure-functions-durable/test_client_compat.py:638

  • Similar to test_get_status_all_returns_wrapped_list, this test validates output on a default RUNNING fake state. durabletask typically doesn't populate serialized_output for RUNNING instances (durabletask/client.py:268-270), so using a COMPLETED fake state here better reflects the terminal-state scenario this shim is meant to preserve (output/custom status/failure details).
async def test_get_status_by_returns_wrapped_list():
    client = _make_client()
    try:
        with patch.object(client, "get_all_orchestration_states",
                          new=AsyncMock(return_value=[_fake_state()])):
            with pytest.warns(DeprecationWarning):
                statuses = await client.get_status_by(
                    runtime_status=[df.OrchestrationRuntimeStatus.Running])
        assert statuses[0].instance_id == "abc"
        assert statuses[0].input_ == {"in": 1}
        assert statuses[0].output == {"out": 2}
        assert statuses[0].custom_status == "cs"

tests/azure-functions-durable/test_client_compat.py:622

  • _fake_state() defaults to a RUNNING instance but the assertions in this test expect an output payload. In durabletask, non-terminal instances typically have serialized_output=None (see durabletask/client.py:268-270), so asserting output for RUNNING here makes the test less representative and potentially brittle. Consider exercising a COMPLETED state when validating output preservation.

This issue also appears on line 627 of the same file.

async def test_get_status_all_returns_wrapped_list():
    client = _make_client()
    try:
        with patch.object(client, "get_all_orchestration_states",
                          new=AsyncMock(return_value=[_fake_state()])):
            with pytest.warns(DeprecationWarning):
                statuses = await client.get_status_all()
        assert len(statuses) == 1
        assert statuses[0].runtime_status == df.OrchestrationRuntimeStatus.Running
        assert statuses[0].input_ == {"in": 1}
        assert statuses[0].output == {"out": 2}
        assert statuses[0].custom_status == "cs"

@berndverst berndverst left a comment

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.

Re-reviewed the follow-up at this exact head. The failed-status output property now preserves the failure message, to_json() reuses the same projection, and both prior threads are resolved. Focused projection probes, unit tests, lint, strict typing, and compatibility checks pass.

@andystaples
andystaples merged commit c595295 into main Jul 29, 2026
25 checks passed
@andystaples
andystaples deleted the andystaples-address-issue-231 branch July 29, 2026 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preserve output and custom status in v1 status query shims

3 participants