Skip to content

Update DurableOrchestrationStatus to reference correct history data key from init argument #621

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

Open
wants to merge 3 commits into
base: v3.x
Choose a base branch
from

Conversation

JPF3N998
Copy link

@JPF3N998 JPF3N998 commented Dec 21, 2024

Addresses #620

What's new?

According to official docs, orchestration history data is returned on the historyEvents key inside the response.

Current implementation prior to these changes are referencing the history key from the response, which doesn't exist leading to setting undefined on the DurableOrchestrationStatus's history attribute.

This PR updates the DurableORchestrationStatus's constructor to pluck the history data from the historyEvents key from the init constructor argument and also update the class attribute history name to historyEvents for consistency and match the official docs.

@JPF3N998 JPF3N998 changed the title Update DurableOrchestrationStatus to reference correct history data key from data.response Update DurableOrchestrationStatus to reference correct history data key from init argument Dec 21, 2024
@cgillum
Copy link
Member

cgillum commented May 15, 2025

/azp run durable-js.public

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@cgillum
Copy link
Member

cgillum commented May 15, 2025

@davidmrdavid or @andystaples - wondering if either of you have context enough to know if these changes are safe to accept? The CI seems happy with them, but I'm not super familiar with how good the coverage is.

@davidmrdavid
Copy link
Collaborator

@cgillum: the change looks right to me. I think someone should run the scenario locally before and after the change as a smoke test, and beyond that this should be good to approve imo.

Thanks @JPF3N998 for your contribution as well!

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.

3 participants