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

V2 API refactors #1293

Open
2 of 13 tasks
mgravell opened this issue Jul 11, 2019 · 10 comments
Open
2 of 13 tasks

V2 API refactors #1293

mgravell opened this issue Jul 11, 2019 · 10 comments
Assignees
Labels
area:api API Additions or Changes breaking-change v3.0 Changes awaiting the next breaking release
Milestone

Comments

@mgravell
Copy link
Member

mgravell commented Jul 11, 2019

So v2 is going to have to happen at some point; we will want to do as much as we can at one "break", so: thoughts here:

Virtually definite

  • unify strong-name
  • remove overload-'splosion caused by gradual growth of overloads
  • move from IDbConnection etc to DbConnection for the async methods
  • make sure CancellationToken is used on all async methods
  • drop TFM support for netstandard1.3
  • remove the SqlClient dependency (need to fix one API only)

Virtually definite (additive; not a break)

  • new IAsyncEnumerable<T> API

Maybe

  • move from IDbConnection etc to DbConnection throughout
  • move from Task<T> to ValueTask<T> throughout
  • move the existing QueryAsync<T> API to IAsyncEnumerable<T> (was Task<IEnumerable<T>>)

Maybe (additive; not a break)

  • add TFM for netcoreapp3.0 and review what we can exploit
  • support for new ADO.NET batching API

Wouldn't rule it out (somewhere between "maybe" and "nope")

  • PooledAwait (internal implementation detail, but adds a dependency; works best with ValueTask<T>, but can have benefits on Task<T> too)

other ideas?

@mgravell
Copy link
Member Author

heh, turns out we've already started this here - but: always good to get another round of input!

@kevin-montrose
Copy link
Contributor

Feels like there might be some value in keeping a QueryAsync<T> (or equivalent) that returns Task<IEnumerable<T>>. The case of "I don't want to block on the query, but I don't want to proceed until I've got all the results" (as compared to "I don't want to block, but can proceed once the first row is available") doesn't seem un(reasonble|common). It kind of maps to buffered/not-buffered, but I don't know that that would make sense as a separate set of methods rather than the parameter it is now.

I, of course, am tempted to want the MarkedSqlString-stuff added (opt-in, naturally) since it's so damn useful. Don't know if there's a clean way to do that though, the SO impl probably has some MS SQL Server assumptions.

@mgravell
Copy link
Member Author

That's actually very relevant, @kevin-montrose - because we could also take the opportunity to clarify buffered Vs non-buffered. Buffered => List<T> / ValueTask<List<T>>, non-buffered => I[Async]Enumerable<T>

@jnm2
Copy link
Contributor

jnm2 commented Jul 11, 2019

There are many times where my method's return type is Task<ImmutableArray<Foo>> and I don't want to buffer to a List<Foo> since I have to do .ToImmutableArray() myself anyway. It would be cool if somehow the immutable array builder could be the buffer itself.

@mgravell
Copy link
Member Author

@jnm2 we'd still have a non-buffered API - it would just be more explicit in the naming; in your case, you'd presumably just write an extension method that wraps that up for you?

@jnm2
Copy link
Contributor

jnm2 commented Jul 11, 2019

That sounds good.

@vdaron
Copy link

vdaron commented Jul 12, 2019

Don't know if it's the correct place, but not having the TypeMapper a singleton would be very helpful. The current behavior doesn't allow your application to be connected to two different databases at the same time.
Maybe attach these TypeMappers on the DbConnection ?
Thanks

@mgravell
Copy link
Member Author

@vdaron valid observation, but not quite sure what that would look like in terms of usage; presumably we'd need some kind of config state, so is that an argument? or a source? i.e. is it

var data = connection.Query<T>("...", ... , someConfigArg: configParam);

or is it:

var data = config.Query<T>(connection, "...", ... );

or is it something else? I think that's a valid discussion, but probably one to split out separately. If there is a case to add it as a parameter, for example, that would be better done "sooner" than "later".

@vdaron
Copy link

vdaron commented Jul 12, 2019

In terms of usage, I would prefer the first one.

One other option would be to have a DapperConnection inheriting from IDbConnection and encapsulating a DbConnection , but it's probably a little too much :-)

@mgravell
Copy link
Member Author

One other option would be to have a DapperConnection inheriting from IDbConnection and encapsulating a DbConnection

My concern there is that a lot of apps allocate lots of connections, which makes that - well, not impossible, but perhaps undesirable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api API Additions or Changes breaking-change v3.0 Changes awaiting the next breaking release
Projects
None yet
Development

No branches or pull requests

5 participants