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

issues(4918): fix resolve scoped service for Elsa.MongoDb.Modules #4924

Merged
merged 2 commits into from
Feb 9, 2024

Conversation

bbenameur
Copy link
Contributor

Releated to this issues: #4918

@sfmskywalker
Copy link
Member

Your PR makes sense, and I think we can get by with a single scope that we create first and then pass down to the individual "create indices" methods. What do you think?

@bbenameur
Copy link
Contributor Author

I agree this is the correct way

@@ -8,9 +8,9 @@ namespace Elsa.MongoDb.Modules.Identity;

internal class CreateIndices : IHostedService
{
private readonly IServiceProvider _serviceProvider;
private readonly IServiceScope _serviceScope;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I am not 100% sure, but if this hosted service remains in memory, so does this child service scope. Therefore, perhaps it is better to not store the child scope asa private field, but rather as a temporary local variable that gets disposed when done with.

E.g.

private readonly IServiceProvider _serviceProvider;

    public CreateIndices(IServiceProvider serviceProvider) => _serviceProvider = serviceProvider;

    public Task StartAsync(CancellationToken cancellationToken)
    {
       using var scope = _serviceProvider.CreateScope();
        return Task.WhenAll(
            CreateWorkflowDefinitionIndices(scope, cancellationToken),
            CreateWorkflowInstanceIndices(scope, cancellationToken));
    }

issues(4918): scope service by instance CreateIndices
Copy link
Member

@sfmskywalker sfmskywalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome 👍🏻

@sfmskywalker sfmskywalker merged commit 3167ae1 into elsa-workflows:main Feb 9, 2024
1 of 2 checks passed
@sfmskywalker sfmskywalker added elsa 3 This issue is specific to Elsa 3 bug Something isn't working labels Feb 9, 2024
@sfmskywalker sfmskywalker added this to the Elsa 3.1 milestone Feb 9, 2024
@bbenameur bbenameur deleted the issues(4918) branch February 12, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working elsa 3 This issue is specific to Elsa 3
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants