Skip to content
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

Fluent configurations #399

Open
BOBx5 opened this issue Sep 3, 2024 · 3 comments
Open

Fluent configurations #399

BOBx5 opened this issue Sep 3, 2024 · 3 comments

Comments

@BOBx5
Copy link

BOBx5 commented Sep 3, 2024

AS-IS

Currently, To use Coravel Queue, I have to use functions below

public void ConfigureServices(IServiceCollection services)
{
    services.AddQueue();
}
public static IApplicationBuilder UseCoravel(this IApplicationBuilder app)
{
    app.ApplicationServices.ConfigureQueue();
    return app;
}

TO-BE

1. Queue Consumption Delay with milliseconds

For better customizable options, I propose that the delay time can be set with milliseconds.

QueuingHost.cs

  • AS-IS

    public Task StartAsync(CancellationToken cancellationToken)
    {
        // ...
        this._timer = new Timer(
            (state) => this._signal.Release(), 
            null, 
            TimeSpan.Zero, 
            TimeSpan.FromSeconds(consummationDelay));
        // ...
    }
  • TO-BE

    public Task StartAsync(CancellationToken cancellationToken)
    {
        // ...
        this._timer = new Timer(
            (state) => this._signal.Release(), 
            null, 
            TimeSpan.Zero, 
            TimeSpan.FromMilliseconds(consummationDelay)); // Change to milliseconds
        // ...
    }

2. Add Extension Method for Queue Consummation Delay (or other options)

public void ConfigureServices(IServiceCollection services)
{
    services.AddQueue()
        .WithConsummationDelay(1000); // 1 second
}
@jamesmh
Copy link
Owner

jamesmh commented Sep 3, 2024

You can already set the consummation delay from your appsettings.json file -> https://docs.coravel.net/Queuing/#adjusting-consummation-delay.

This would definitely be a breaking change, since right now in that file 1 represents 1 second. So we'd need to figure out what the migration path would be.

Any ideas on how we could change the delay to use ms but not break existing usages of this feature?

@BOBx5
Copy link
Author

BOBx5 commented Sep 4, 2024

What if add configuration property on Coravel:Queue:ConsummationDelayUnit ?

Modify QueuingHost.cs like below:

internal class QueuingHost : IHostedService, IDisposable
{

    private IConfiguration _configuration;
    // ...

    public Task StartAsync(CancellationToken cancellationToken)
    {
        TimeSpan consummationDelay = GetConsummationDelay();

        this._timer = new Timer((state) => this._signal.Release(), null, TimeSpan.Zero, consummationDelay);
        Task.Run(ConsumeQueueAsync);
        return Task.CompletedTask;
    }

    // Change return type from `int` to `TimeSpan` 
    private TimeSpan GetConsummationDelay()
    {
        string delayUnit = GetConsummationDelayUnit();
        int delayValue = GetConsummationDelayValue(delayUnit);

        switch (delayUnit)
        {
            case "Milliseconds":
                return TimeSpan.FromMilliseconds(delayValue);
            case "Seconds":
            default:
                return TimeSpan.FromSeconds(delayValue);
        }
    }

    // Get delay unit ("Milliseconds" or "Seconds").
    // This can be improved by using an enum.
    private string GetConsummationDelayUnit()
    {
        var delayUnitSection = this._configuration.GetSection("Coravel:Queue:ConsummationDelayUnit");
        return delayUnitSection.Value ?? "Seconds";
    }

    private int GetConsummationDelayValue(string delayUnit)
    {
        var delayValueSection = this._configuration.GetSection("Coravel:Queue:ConsummationDelay");
        if (int.TryParse(delayValueSection.Value, out int parsedDelay))
        {
            return parsedDelay;
        }

        switch (delayUnit)
        {
            case "Milliseconds":
                return 30000;

            case "Seconds":
            default:
                return 30;
        }
    }
}

@jamesmh
Copy link
Owner

jamesmh commented Sep 5, 2024

Cool. Do you want to create an PR for this?

Could you also add some tests for these methods? You can make the new method public and test it in the unit test project by adding something like this to the top of the class: [assembly:InternalsVisibleTo("NameOfYourUnitTestProject")].

Thanks!

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

No branches or pull requests

2 participants