-
Notifications
You must be signed in to change notification settings - Fork 53
Allow completed Tasks to expose their results #753
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
Merged
davidmrdavid
merged 1 commit into
Azure:dajusto/external-contrib-get-taskResult
from
oobegreg:dev
Mar 9, 2022
Merged
Allow completed Tasks to expose their results #753
davidmrdavid
merged 1 commit into
Azure:dajusto/external-contrib-get-taskResult
from
oobegreg:dev
Mar 9, 2022
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AnatoliB
reviewed
Feb 8, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you! I would also love to see some E2E test coverage for the new command.
I'll be merging this into a separate branch that I own, so that I can add the E2E tests |
davidmrdavid
added a commit
that referenced
this pull request
Apr 7, 2022
fix typo DurableSDK now compiles by itself Allow ExternalSDK to handle orchestration document next steps allow external SDK to set the user-code's input. Still need to refactor this logic for the worker to continue working with old SDK add import module supress traces avoid nullptr pass tests fix E2E tests develop E2E tests Enabled external durable client (#765) Co-authored-by: Michael Peng <michaelpeng@microsoft.com> bindings work conditional binding intialization conditional import Added exception handling logic Revert durableController name to durableFunctionsUtils Ensure unit tests are functioning properly Corrected unit test names Turned repeated variables in unit tests into static members Fixed issue with building the worker Fix E2E test Fixed unit test setup Fixed another unit test setup Remove string representation of booleans patch e2e test remove typo in toString Update PowerShell language worker pipelines (#750) * Install .Net to a global location * Remove .Net installation tasks * Update install .Net 6 task * Update Windows image to use windows-latest Make throughput warning message visible for tooling diagnosis (#757) Update grpc.tools to version 2.43.0 Update Google.Protobuf.Tools to version 3.19.4 Revert "Update Google.Protobuf.Tools to version 3.19.4" This reverts commit bcbd022. Revert "Update grpc.tools to version 2.43.0" This reverts commit ccb323a. Update Google.Protobuf to 3.19.4 and grpc.tools to 2.43.0 (#762) * Update grpc.tools to version 2.43.0 * Update Google.Protobuf.Tools to version 3.19.4 Switch from Grpc.Core to Grpc.Net.Client (#758) * Upgraded protobuf versions and removed Grpc.Core dependency * Updated channel and option types used * Change channel credentials * Added http prefix to url * Add valid URL check and explicitly include credentials Update pipeline logic to generate the SBOM for release builds (#767) Return results from Start-DurableExternalEventListener (#685) (#753) Co-authored-by: Greg Roll <Greg.Roll@oobe.com.au> add e2e test for GetTaskResult parametrize test patch new e2e test patch external contrib fix typo in test comment changes Adds IExternalInvoker (#776) * Define a contract for the external invoker * Remove extraneous comments and variables rename hasOrchestrationContext to hasInitializedDurableFunction remove outdated TODO comment remove now unused function - CreateOrchestrationBindingInfo Allow worker to read results directly from the external SDK (#777) comment out external SDK path
davidmrdavid
added a commit
that referenced
this pull request
Apr 7, 2022
fix typo DurableSDK now compiles by itself Allow ExternalSDK to handle orchestration document next steps allow external SDK to set the user-code's input. Still need to refactor this logic for the worker to continue working with old SDK add import module supress traces avoid nullptr pass tests fix E2E tests develop E2E tests Enabled external durable client (#765) Co-authored-by: Michael Peng <michaelpeng@microsoft.com> bindings work conditional binding intialization conditional import Added exception handling logic Revert durableController name to durableFunctionsUtils Ensure unit tests are functioning properly Corrected unit test names Turned repeated variables in unit tests into static members Fixed issue with building the worker Fix E2E test Fixed unit test setup Fixed another unit test setup Remove string representation of booleans patch e2e test remove typo in toString Update PowerShell language worker pipelines (#750) * Install .Net to a global location * Remove .Net installation tasks * Update install .Net 6 task * Update Windows image to use windows-latest Make throughput warning message visible for tooling diagnosis (#757) Update grpc.tools to version 2.43.0 Update Google.Protobuf.Tools to version 3.19.4 Revert "Update Google.Protobuf.Tools to version 3.19.4" This reverts commit bcbd022. Revert "Update grpc.tools to version 2.43.0" This reverts commit ccb323a. Update Google.Protobuf to 3.19.4 and grpc.tools to 2.43.0 (#762) * Update grpc.tools to version 2.43.0 * Update Google.Protobuf.Tools to version 3.19.4 Switch from Grpc.Core to Grpc.Net.Client (#758) * Upgraded protobuf versions and removed Grpc.Core dependency * Updated channel and option types used * Change channel credentials * Added http prefix to url * Add valid URL check and explicitly include credentials Update pipeline logic to generate the SBOM for release builds (#767) separate DF SDK classes from DF worker classes fix typo DurableSDK now compiles by itself Allow ExternalSDK to handle orchestration document next steps allow external SDK to set the user-code's input. Still need to refactor this logic for the worker to continue working with old SDK add import module supress traces avoid nullptr pass tests fix E2E tests develop E2E tests Enabled external durable client (#765) Co-authored-by: Michael Peng <michaelpeng@microsoft.com> bindings work conditional binding intialization conditional import Added exception handling logic Revert durableController name to durableFunctionsUtils Ensure unit tests are functioning properly Corrected unit test names Turned repeated variables in unit tests into static members Fixed issue with building the worker Fix E2E test Fixed unit test setup Fixed another unit test setup Remove string representation of booleans patch e2e test remove typo in toString Update PowerShell language worker pipelines (#750) * Install .Net to a global location * Remove .Net installation tasks * Update install .Net 6 task * Update Windows image to use windows-latest Make throughput warning message visible for tooling diagnosis (#757) Update grpc.tools to version 2.43.0 Update Google.Protobuf.Tools to version 3.19.4 Revert "Update Google.Protobuf.Tools to version 3.19.4" This reverts commit bcbd022. Revert "Update grpc.tools to version 2.43.0" This reverts commit ccb323a. Update Google.Protobuf to 3.19.4 and grpc.tools to 2.43.0 (#762) * Update grpc.tools to version 2.43.0 * Update Google.Protobuf.Tools to version 3.19.4 Switch from Grpc.Core to Grpc.Net.Client (#758) * Upgraded protobuf versions and removed Grpc.Core dependency * Updated channel and option types used * Change channel credentials * Added http prefix to url * Add valid URL check and explicitly include credentials Update pipeline logic to generate the SBOM for release builds (#767) Return results from Start-DurableExternalEventListener (#685) (#753) Co-authored-by: Greg Roll <Greg.Roll@oobe.com.au> add e2e test for GetTaskResult parametrize test patch new e2e test patch external contrib fix typo in test comment changes Adds IExternalInvoker (#776) * Define a contract for the external invoker * Remove extraneous comments and variables rename hasOrchestrationContext to hasInitializedDurableFunction remove outdated TODO comment remove now unused function - CreateOrchestrationBindingInfo Allow worker to read results directly from the external SDK (#777) comment out external SDK path
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contribution by @oobegreg, thank you!
Issue describing the changes in this PR
resolves #685
Currently, users do not have a way of obtaining the result of a DF Task that has already completed. This means that if a user did not assign a Task onto a variable when first invoking it, then they have no way of accessing its result.
This is an urgently missing feature and @oobegreg is proposing a CmdLet named
Get-DurableTaskResult
that allows us to do just that.In general, this serves the purpose of performing an
await
(oryield
in Python and JS) on a completed Durable Task, because in those languages re-awaiting a completed Task is a supported means of extracting that Task's result. Another way to do this is to expose a.result
property on theDurableTask
class and have users access that. However, I think this CmdLet-based approach is also sound, and I don't see why we couldn't have both given that users in other PLs can both access their classes'.result
property and callyield/await
for the same effect.My recommendation is to merge this contribution, but I would like to get everyone's feedback first. Thanks!
Pull request checklist
release_notes.md
Additional information
Additional PR information