-
Notifications
You must be signed in to change notification settings - Fork 54
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
Prepare Worker for External DF SDK support (and a few DF bug fixes) #746
Closed
Closed
Changes from all commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
4423d13
separate DF SDK classes from DF worker classes
davidmrdavid 9d73110
fix typo
davidmrdavid 2fcbfae
DurableSDK now compiles by itself
davidmrdavid f43f350
Allow ExternalSDK to handle orchestration
davidmrdavid 092e333
document next steps
davidmrdavid 506e4f7
allow external SDK to set the user-code's input. Still need to refact…
davidmrdavid 7a38ec8
add import module
davidmrdavid 9b2cf42
supress traces
davidmrdavid 47caee9
avoid nullptr
davidmrdavid 1010f97
pass tests
davidmrdavid d916d58
fix E2E tests
davidmrdavid 946ea37
develop E2E tests
davidmrdavid 64b6c49
Enabled external durable client (#765)
davidmrdavid 9e7cf96
bindings work
davidmrdavid 92f113e
conditional binding intialization
davidmrdavid e5843ce
conditional import
davidmrdavid 2ee6b96
Added exception handling logic
michaelpeng36 ae28f6e
Ensure unit tests are functioning properly
michaelpeng36 94f995d
Corrected unit test names
michaelpeng36 96f6ee7
Turned repeated variables in unit tests into static members
michaelpeng36 1cb30b8
Revert durableController name to durableFunctionsUtils
michaelpeng36 ff5102e
Merge remote-tracking branch 'origin/michaelpeng/get-durable-tests-wo…
davidmrdavid b8c6a6a
Fixed issue with building the worker
michaelpeng36 baae3cc
Fix E2E test
michaelpeng36 6feb7fe
Fixed unit test setup
michaelpeng36 94d83ae
Fixed another unit test setup
michaelpeng36 d11a561
Remove string representation of booleans
michaelpeng36 0e0c077
patch e2e test
davidmrdavid 1c4a7ae
remove typo in toString
davidmrdavid dc0ed38
Merge branch 'dev' of https://github.com/Azure/azure-functions-powers…
davidmrdavid 3bd961c
Return results from Start-DurableExternalEventListener (#685) (#753)
davidmrdavid caa5176
add external contrib
davidmrdavid 932e124
add e2e test for GetTaskResult
davidmrdavid 3d1a7b4
parametrize test
davidmrdavid adec8fa
patch new e2e test
davidmrdavid 8d5d9bf
patch external contrib
davidmrdavid 8351653
fix typo in test
davidmrdavid 7232e58
comment changes
davidmrdavid d9b3baf
Adds IExternalInvoker (#776)
michaelpeng36 edf3011
rename hasOrchestrationContext to hasInitializedDurableFunction
davidmrdavid aca4c0b
remove outdated TODO comment
davidmrdavid e5ca2ba
remove now unused function - CreateOrchestrationBindingInfo
davidmrdavid f69b146
Allow worker to read results directly from the external SDK (#777)
michaelpeng36 4387799
comment out external SDK path
davidmrdavid a9a0a6e
change serialization
davidmrdavid 12ccdee
Merge branch 'dev' of https://github.com/Azure/azure-functions-powers…
davidmrdavid 5f862e6
restore orchestrationInvoker and powershellServices
davidmrdavid File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,10 +6,12 @@ | |
namespace Microsoft.Azure.Functions.PowerShellWorker.Durable | ||
{ | ||
using System; | ||
using System.Collections; | ||
using System.Collections.Generic; | ||
using System.Management.Automation; | ||
using System.Threading; | ||
using Microsoft.Azure.Functions.PowerShellWorker.Durable.Tasks; | ||
using Utility; | ||
using Microsoft.PowerShell.Commands; | ||
|
||
internal class DurableTaskHandler | ||
{ | ||
|
@@ -83,7 +85,7 @@ public void StopAndInitiateDurableTaskOrReplay( | |
retryOptions.MaxNumberOfAttempts, | ||
onSuccess: | ||
result => { | ||
output(TypeExtensions.ConvertFromJson(result)); | ||
output(ConvertFromJson(result)); | ||
}, | ||
onFailure); | ||
|
||
|
@@ -232,15 +234,41 @@ private static object GetEventResult(HistoryEvent historyEvent) | |
|
||
if (historyEvent.EventType == HistoryEventType.TaskCompleted) | ||
{ | ||
return TypeExtensions.ConvertFromJson(historyEvent.Result); | ||
return ConvertFromJson(historyEvent.Result); | ||
} | ||
else if (historyEvent.EventType == HistoryEventType.EventRaised) | ||
{ | ||
return TypeExtensions.ConvertFromJson(historyEvent.Input); | ||
return ConvertFromJson(historyEvent.Input); | ||
} | ||
return null; | ||
} | ||
|
||
public static object ConvertFromJson(string json) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This used to exist in |
||
{ | ||
object retObj = JsonObject.ConvertFromJson(json, returnHashtable: true, error: out _); | ||
|
||
if (retObj is PSObject psObj) | ||
{ | ||
retObj = psObj.BaseObject; | ||
} | ||
|
||
if (retObj is Hashtable hashtable) | ||
{ | ||
try | ||
{ | ||
// ConvertFromJson returns case-sensitive Hashtable by design -- JSON may contain keys that only differ in case. | ||
// We try casting the Hashtable to a case-insensitive one, but if that fails, we keep using the original one. | ||
retObj = new Hashtable(hashtable, StringComparer.OrdinalIgnoreCase); | ||
} | ||
catch | ||
{ | ||
retObj = hashtable; | ||
} | ||
} | ||
|
||
return retObj; | ||
} | ||
|
||
private void InitiateAndWaitForStop(OrchestrationContext context) | ||
{ | ||
context.OrchestrationActionCollector.Stop(); | ||
|
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
// | ||
// Copyright (c) Microsoft. All rights reserved. | ||
// Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
// | ||
|
||
namespace Microsoft.Azure.Functions.PowerShellWorker.Durable | ||
{ | ||
using System; | ||
using System.Collections; | ||
using System.Management.Automation; | ||
|
||
internal class ExternalInvoker : IExternalInvoker | ||
{ | ||
private readonly Func<PowerShell, object> _externalSDKInvokerFunction; | ||
|
||
public ExternalInvoker(Func<PowerShell, object> invokerFunction) | ||
{ | ||
_externalSDKInvokerFunction = invokerFunction; | ||
} | ||
|
||
public Hashtable Invoke(IPowerShellServices powerShellServices) | ||
{ | ||
return (Hashtable)_externalSDKInvokerFunction.Invoke(powerShellServices.GetPowerShell()); | ||
} | ||
} | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// | ||
// Copyright (c) Microsoft. All rights reserved. | ||
// Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
// | ||
|
||
namespace Microsoft.Azure.Functions.PowerShellWorker.Durable | ||
{ | ||
using System.Collections; | ||
|
||
// Represents a contract for the | ||
internal interface IExternalInvoker | ||
{ | ||
// Method to invoke an orchestration using the external Durable SDK | ||
Hashtable Invoke(IPowerShellServices powerShellServices); | ||
} | ||
} |
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
// | ||
// Copyright (c) Microsoft. All rights reserved. | ||
// Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
// | ||
|
||
using System.Collections; | ||
using System.Collections.ObjectModel; | ||
|
||
namespace Microsoft.Azure.Functions.PowerShellWorker.Durable | ||
{ | ||
using System.Management.Automation; | ||
|
||
internal static class PowerShellExtensions | ||
{ | ||
public static void InvokeAndClearCommands(this PowerShell pwsh) | ||
{ | ||
try | ||
{ | ||
pwsh.Invoke(); | ||
} | ||
finally | ||
{ | ||
pwsh.Streams.ClearStreams(); | ||
pwsh.Commands.Clear(); | ||
} | ||
} | ||
|
||
public static void InvokeAndClearCommands(this PowerShell pwsh, IEnumerable input) | ||
{ | ||
try | ||
{ | ||
pwsh.Invoke(input); | ||
} | ||
finally | ||
{ | ||
pwsh.Streams.ClearStreams(); | ||
pwsh.Commands.Clear(); | ||
} | ||
} | ||
|
||
public static Collection<T> InvokeAndClearCommands<T>(this PowerShell pwsh) | ||
{ | ||
try | ||
{ | ||
var result = pwsh.Invoke<T>(); | ||
return result; | ||
} | ||
finally | ||
{ | ||
pwsh.Streams.ClearStreams(); | ||
pwsh.Commands.Clear(); | ||
} | ||
} | ||
|
||
public static Collection<T> InvokeAndClearCommands<T>(this PowerShell pwsh, IEnumerable input) | ||
{ | ||
try | ||
{ | ||
var result = pwsh.Invoke<T>(input); | ||
return result; | ||
} | ||
finally | ||
{ | ||
pwsh.Streams.ClearStreams(); | ||
pwsh.Commands.Clear(); | ||
} | ||
} | ||
} | ||
} |
Oops, something went wrong.
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.
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.
These two were removed since they're incompatible with the external SDK. In addition, other DF SDKs don't perform this validation steps. If we want them, they ought to exist outside the DF SDKs.
I do agree that these are useful validations to have though, but if we perform these checks in the DF SDK components, then we're creating a strong dependency with worker-specific utilities.
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.
The intent of
ActivityInvocationTask.ValidateTask
is to provide user-friendly error messages when the specified activity function does not exits (for example, because of a typo in the name). Without this code, the error messages are really confusing and unhelpful. If you remove it, we need an adequate replacement.