search

Electron100 / butane

An ORM for Rust with a focus on simplicity and on writing Rust, not SQL
Apache License 2.0
97 stars 13 forks source link

`butane` + async runtime #13

Open TmLev opened 3 years ago

TmLev commented 3 years ago

Is it possible to use butane with asynchronous runtime like, for example, tokio?

Electron100 commented 3 years ago

Not directly, aside from using something like tokio::task::spawn_blocking of course. I'm not opposed to some deeper async integration, but it's not something that I'm expecting to implement soon

Electron100 commented 3 years ago

Closing this out as it's not something I'm expecting to do any time soon. I'd be open to a pull request for this, although it would likely only be applicable to some backends, since e.g. sqlite is always going to be based on libsqlite which isn't async-aware.

ibraheemdev commented 3 years ago

The sqlite backend would still have to expose async methods that do something like spawn_blocking.

Electron100 commented 3 years ago

@ibraheemdev sure, but the consumer of Butane could just as well make the spawn_blocking calls themselves. That's the same approach taken with Diesel, as in this Actix Web example https://actix.rs/docs/databases/.

ibraheemdev commented 3 years ago

Yeah, I was just saying that if you wanted to expose an async API, ideally everything should be usable directly in an async context.

Electron100 commented 1 year ago

Reactivating as this is under active consideration

jayvdb commented 7 months ago

I've merged master into the latest async branch , disabling r2d2 feature. https://github.com/jayvdb/butane/tree/async-no-r2d2-jv-merge-master

Electron100 commented 7 months ago

@jayvdb remind me what is the intended difference between that branch and async-wip? That also has r2d2 currently disabled.

I've just added async_checklist.md to attempt to track the remaining work on that branch.

jayvdb commented 5 months ago

I think I removed r2r2 from the features list so that I could use --all-features, and it has a recent master merged into it.

jayvdb commented 4 months ago

https://github.com/nvksv/maybe-async-cfg/pull/4 has been merged and released, which we can use on the butane branch to reduce the dependencies.

Electron100 commented 2 weeks ago

Hopefully better late than never, as of this week, I believe the async-wip branch is finally functional and API-stable (although that's not a guarantee). Remaining tasks are tracked in https://github.com/Electron100/butane/blob/async-wip/async_checklist.md. The biggest outstanding items are to properly prove/ensure soundness in some unsafe code used to bridge between sync and async when necessary and to provide support for async connections pools (deadpool and/or bb8, although this is not blocking as it could also be done out of tree).

If anybody is interested in giving it a whirl, feedback is welcome.

RobertoMaurizzi commented 2 weeks ago

About deadpool (and sorry if this is obviously wrong/impossible :sweat_smile: ) but wouldn't https://crates.io/crates/deadpool-postgres be usable if we can give it a tokio_postgres::Config? However, I just started checking the code but I can't see any use of that Config object, so maybe not...

Electron100 commented 2 weeks ago

Butane doesn't expose its Postgres implementation. From a quick scan of the deadpool docs, the way to do deadpool integration would be to create a type like butane::db::deadpool::ConnectionManager which is constructed with a butane ConnectionSpec and implements Manager in deadpool::managed - Rust https://docs.rs/deadpool/latest/deadpool/managed/trait.Manager.html to create a new connection from that spec on demand. Should be straightforward but I'm traveling for work this week so unlikely to get to it until next week at the earliest.

On Mon, Oct 21, 2024 at 7:08 AM Roberto Maurizzi @.***> wrote:

About deadpool (and sorry if this is obviously wrong/impossible 😅 ) but wouldn't https://crates.io/crates/deadpool-postgres be usable if we can give it a tokio_postgres::Config? However, I just started checking the code but I can't see any use of that Config object, so maybe not...

— Reply to this email directly, view it on GitHub https://github.com/Electron100/butane/issues/13#issuecomment-2426357538, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABKVBR4KWCSQL6GM2I3ZNDZ4TOB3AVCNFSM5BWHR35KU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TENBSGYZTKNZVGM4A . You are receiving this because you modified the open/close state.Message ID: @.***>

jayvdb commented 1 week ago

@Electron100 , thanks for finishing that off. IMO it would be good to push the async branch to master. Squashed under your id would be my pref as the history is quite messy? You've done all the hard bits.

Then the remaining TODOs can be done by separate people as normal PRs.

Maybe include a note at the top of the README that master is unstable atm while we finish the async work.

Electron100 commented 1 week ago

Good point. I've created the PR. I'd like to give it a once over myself over the next few days (and of course other reviews are very welcome) to look for anything egregiously bad/incomplete before merging, but will plan to merge it within the week (and yes, I'll squash).

Electron100 commented 1 week ago

PR is in master now! I'm waiting to close this issue fully until we've done some of the tracked cleanup.