conductor-sdk / conductor-csharp Goto Github PK

In Conductor.Client.Extensions.DependencyInjectionExtensions.AddConductorWorker()
The binding of IConductorWorkerRestClient was removed by commenting it out
// services.AddTransient<IConductorWorkerRestClient, ConductorWorkerRestClient>();

But the client is needed by WorkflowTaskExecutor which is bound a few lines further down
services.AddTransient<IWorkflowTaskExecutor, WorkflowTaskExecutor>();

This leads to the hosted service hanging during Start because WorkflowTaskCoordinator can't get the implementation from the serviceProvider

• Workflow creation API received a new parameter: overwrite: bool

Resources

Go example: https://github.com/conductor-sdk/conductor-go/blob/v1.2.3/sdk/workflow/executor/executor.go#L53

I am upgrading my application from an older version of the SDK.
We are using a Background Service and IWorkflowTaskCoordinator to execute IWorkflowTask.

Much to my surprise, the Execute method of IWorkflowTask has been made synchronous, which is a real pain, since many of our workers are performing tasks on databases, making http requests, and other stuff that would naturally be async.

Since polling for tasks is also an HTTP request, I guess that the whole execution must already be in an async context, so it must have required some work to remove it from IWorkflowTask. In other words I guess this must have been done for a very good reason, which I have a hard time figuring out what is.

Can you explain the reason why this was decided or if there is another way that executing async tasks is meant to be performed?
Right now I feel like I have to choose between sticking to an obsolete version or write my own framework. I don't like either of those paths.

There appears to be an endpoint to update Workflow variables on the Swagger API page, but it's not (from what we could find) supported in the SDK, we had to add a separate method in our project to call the API directly via HttpClient rather than using the SDK. Can this be added to the SDK please? Thanks!

When referencing the conductor-csharp SDK below, I get:

An error occurred while starting the application.
MissingMethodException: Method not found: 'Void RestSharp.RestClient..ctor(System.String)'.
Conductor.Client.ApiClient..ctor()

public WorkflowService(string url, OrkesAuthenticationSettings settings)
{
--> var config = new Configuration { BasePath = url, AuthenticationSettings = settings };
_workflowClient = config.GetClient();
}

I've confirmed url and settings are correct. When I load the SDK source directly, it gives the same error.

Please advise, Thanks!

I would like some interfaces to support tasks/async methods. Right now the main thread is blocked by any network call. This is a problem for using the api client. Workers can also be made async as well for pull loop would allow a cancelation token to be used so the worker can exit gracefully.

The Microsoft docs they use the async methods to avoid threading issues.
https://learn.microsoft.com/en-us/aspnet/core/fundamentals/host/hosted-services?view=aspnetcore-7.0&tabs=visual-studio

Related issue: https://github.com/orkes-io/orkes-conductor/issues/111

Evaluate behavior on invalid credentials:

• Should workers keep polling with invalid token?

@manan164 can you please look into this?

I'm using the latest conductor-csharp version 0.1.1

References:

• Go SDK
• Python SDK

It is working fine when I use Target Framework as .NET core 3.1).

Now, When I upgrade the .NET framework to .NET 6.0, I am getting 403 error.
StatusCode: 403, ReasonPhrase: 'Forbidden', Version: 1.1, Content: System.Net.Http.HttpConnectionResponseContent, Headers:
{
Date: Fri, 14 Oct 2022 16:42:51 GMT
Connection: keep-alive
Via: HTTP/1.1 m_proxy_mum1
Content-Length: 0
}
var response = await httpClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false);

According to the readme, authentication settings are optional. However, whenever Configuration.AccessToken is accessed, for example when polling for tasks, an exception is eventually thrown at

dbug: Conductor.Client.Worker.WorkflowTaskExecutor[0]
      [DESKTOP-******] worker error: Value cannot be null. (Parameter 'key'), taskName: qwerty, domain: , batchSize: 16

Would it be possible to separate the responsibilities of task queue polling and task execution?

"As a developer, I want to use Azure or AWS serverless (functions/lambda) to periodically check the task queue and execute registered tasks. I want to set a periodic timer with the cloud platform to wake my conductor-sdk code and directly call an interface that will check for new work and dispatch the registered tasks."

Some apps will use the scheduler (background worker) which is part of the SDK, others will have their own trigger.

There should be a way to configure-services without the built-in scheduler.

I'd take a crack at it, but it seems there is a refactor already in flight, #37 .

See:

Experimentally, I found these values to be wrong. Instead, I found them all to be one value lower than the values in code. The values should be zero based, not one based. What I think the correct code is:

        [JsonConverter(typeof(StringEnumConverter))]
        public enum StatusEnum
        {
            /// <summary>
            /// Enum INPROGRESS for value: IN_PROGRESS
            /// </summary>
            [EnumMember(Value = "IN_PROGRESS")]
            INPROGRESS = 0,
            /// <summary>
            /// Enum FAILED for value: FAILED
            /// </summary>
            [EnumMember(Value = "FAILED")]
            FAILED = 1,
            /// <summary>
            /// Enum FAILEDWITHTERMINALERROR for value: FAILED_WITH_TERMINAL_ERROR
            /// </summary>
            [EnumMember(Value = "FAILED_WITH_TERMINAL_ERROR")]
            FAILEDWITHTERMINALERROR = 2,
            /// <summary>
            /// Enum COMPLETED for value: COMPLETED
            /// </summary>
            [EnumMember(Value = "COMPLETED")]
            COMPLETED = 3,
        }

See this line for details.

There is no way to supply a custom serializer in ApiClient. While there exists the 'serializerSettings' member, there are two issues with this:

1. this is not use as input to JsonConvert.Serialize() during serialization
2. the serializerSettings member is private and there is no way of setting it in a constructor.

Additionally, one would think that ApiClient.RestClient.UseSerializer<T> (or the Newtonsoft extension UseNewtonsoftJson) would have an effect but this is completely unused in ApiClient.

Proposed fixes:

1. Add a ctor to allow overload for serialization settings and use that.
2. Use the serializer in the RestClient

Note: callers can work around this by configuring newtonsoft's default serializer options but this is error prone and affects the whole process.

I have come across a conflict between System.Threads.Tasks.Task and Conductor.Client.Models.Task The class in question is here:

Can it be renamed to something else?

When I configed the sleepInterval of the worker, it is not setting the sleepInterval property in the client, it is always using the default value to poll the task queue 1000ms

     var configuration = new Conductor.Client.Configuration(
                        defaultHeader: new ConcurrentDictionary<string, string>(),
                        basePath: $"{config["ConductorEndpoint"]}/api")
                    {
                        // By looking at the Conductor workflow c# sdk source code, SleepInterval should increase poll interval, but it isn't working.
                        ConcurrentWorkers = 1,
                        SleepInterval = 5_000,
                    };

See this code

The callbackAfterSeconds.Value will throw if null yet the param is defined as long? callbackAfterSeconds = null