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

Use Stream<Row> in API #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use Stream<Row> in API #61

wants to merge 1 commit into from

Conversation

andygrove
Copy link
Contributor

No description provided.

@sd2k
Copy link

sd2k commented Jan 27, 2020

@andygrove I tried to use a bunch of associated types on the various traits and managed to get the trait part to compile, although I haven't actually tried implementing the trait on the various database structs yet. I'm not sure if it's what you decided on in the end (following the chat in Discord) but maybe it's useful! sd2k@2adb34a

@andygrove
Copy link
Contributor Author

andygrove commented Jan 27, 2020

@sd2k This is awesome :-) I was hoping someone would jump in and show me the way ... I think I chose the wrong database drivers to integrate with so we should perhaps look at tokio-postgres as the first one to implement with these new traits?

@sd2k
Copy link

sd2k commented Jan 27, 2020

Just taking a look at tokio-postgres. It immediately requires a change to the traits to make Driver::connect async, since the tokio_postgres::connect is async. Alternatively we could make the driver block on the creation of the connection. I've not got much experience with this sort of thing, but my hunch is that Drivers aren't created and destroyed regularly, so that may be acceptable? Never mind, we need to spawn the connection into an ongoing runtime so the method will still need to be async!

@sd2k
Copy link

sd2k commented Jan 28, 2020

This appears to be close to compiling now: sd2k@a483a82. I can't figure out how to make the params in execute_query Send which is blocking at the minute. I had to resort to using an Arc<tokio_postgres::Client> rather than tying the lifetime of TokioPostgresStatement to that of the client, but it probably won't matter realistically!

@sd2k
Copy link

sd2k commented Jan 28, 2020

By some absolute miracle I got it to compile. Check the tests for rdbc-tokio-postgres for examples! There are an awful lot of unwraps in the tests due to the amount of optional/result types returned by the methods, but I'm not sure it's avoidable - they're all valid from what I can tell.

I haven't even touched the other databases and I'm not completely sure how they'll work with the async traits.

Latest commit is here: sd2k@0993910

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

Successfully merging this pull request may close these issues.

2 participants