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

IAsyncReader CancellationToken support #638

Open
dje1990 opened this issue May 1, 2022 · 4 comments
Open

IAsyncReader CancellationToken support #638

dje1990 opened this issue May 1, 2022 · 4 comments

Comments

@dje1990
Copy link

dje1990 commented May 1, 2022

Hi,
It looks like IAsyncReader is a good candidate to add CancellationToken support:
Task<bool> ReadAsync(CancellationToken cancellationToken);

Then the first lines of the implementations of this method can check if cancellation has been requested, and call the dispose method of the IAsyncReader implementation:

        if (cancellationToken.IsCancellationRequested)
        {
            this.Dispose();
            cancellationToken.ThrowIfCancellationRequested();
        }
        else
        {
            ....
        }

A simple example of how this would make using PetaPoco more elegant,
Instead of:

    public static async IAsyncEnumerable<TProjection> QueryAsyncEnumerable<TProjection>(this IDatabase database,
        [EnumeratorCancellation] CancellationToken cancellationToken, string sql, params object[] args)
    {
        using var asyncReader =
            await database.QueryAsync<TProjection>(cancellationToken, sql, args).ConfigureAwait(false);

        if (cancellationToken.IsCancellationRequested)
        {
            asyncReader.Dispose();
            cancellationToken.ThrowIfCancellationRequested();
        }
        else
        {
            // todo: why does ReadAsync not accept a cancellationToken ? then remove these two ThrowIfCancellationRequested checks
            // todo: see https://github.com/CollaboratingPlatypus/PetaPoco/issues/638
            while (await asyncReader.ReadAsync().ConfigureAwait(false))
            {
                yield return asyncReader.Poco;

                if (!cancellationToken.IsCancellationRequested) continue;
                asyncReader.Dispose();
                cancellationToken.ThrowIfCancellationRequested();
            }
        }
    }

Use the new CancellationToken support:

    public static async IAsyncEnumerable<TProjection> QueryAsyncEnumerable<TProjection>(this IDatabase database,
        [EnumeratorCancellation] CancellationToken cancellationToken, string sql, params object[] args)
    {
        using var asyncReader = await database.QueryAsync<TProjection>(cancellationToken, sql, args).ConfigureAwait(false);

        while (await asyncReader.ReadAsync(cancellationToken).ConfigureAwait(false))
        {
            yield return asyncReader.Poco;
        }
    }

Thanks for your help and support.

@pleb
Copy link
Member

pleb commented May 6, 2022

The async methods were added before IAsyncEnumerable was added to the framework. It would be nice to add support for IAsyncEnumerable and update the current methods to use IAsyncEnumerable internally. The only problem would be the current target is netstandard 2.0 and IAsyncEnumerable is supported in netstandard 2.1 and above.

🤔 Maybe time for a version bump

@pleb
Copy link
Member

pleb commented May 6, 2022

Oh, I didn't realise @asherber added support for netstandard 2.1... well this changes my view ;)

@dje1990
Copy link
Author

dje1990 commented May 6, 2022

Hi Wade,
Thanks for your response.
Adding support for IAsyncEnumerable would indeed resolve my original need, so that's great news if it's something that can be considered now that netstandard 2.1 is supported.
I saw the use of IAsyncEnumerable was previously discussed before on #600
I opened this ticket specifically about adding CancellationToken support to the existing IAsyncReader interface, which I think would still be a separate change to adding IAsyncEnumerable (and wouldn't require netstandard 2.1 support I think?). (My example IAsyncReader to IAsyncEnumerable extension method was just an example of how CancellationToken support for IAsyncReader would be useful). However I would vote to prioritise adding IAsyncEnumerable as you suggested if it's possible, rather than adding CancellationToken support to the IAsyncReader interface.
Thanks for your help.

@asherber
Copy link
Collaborator

asherber commented May 6, 2022

Although PP targets 2.1 as well as 2.0, changing the method signature for 2.1 would involve more #ifs, and I think would be a breaking change anyway -- an application targeting net5.0 (for example) which used to link to the netstandard2.0 library would now link to the netstandard2.1 library and see the new methods in place of the old.

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

3 participants