-
Notifications
You must be signed in to change notification settings - Fork 602
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
Implement async pagination #10021
Implement async pagination #10021
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left some comments that aren't blockers.
Aside from the lifetime part, which I'm unsure about it, this PR looks great to me! And since it compiles and passes the test cases, I'm leaning towards accepting it. Awesome job! 👍
pub fn load<'a, U>( | ||
self, | ||
conn: &'a mut AsyncPgConnection, | ||
) -> BoxFuture<'a, QueryResult<Paginated<U>>> | ||
where | ||
Self: LoadQuery<'a, Conn, WithCount<U>>, | ||
Self: diesel_async::methods::LoadQuery<'a, AsyncPgConnection, WithCount<U>>, | ||
T: 'a, | ||
U: Send + 'a, | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I admit that I'm unsure about whether the lifetime 'a
here remains exactly the same as 'conn
and 'query
in diesel-async
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no clue. I followed the suggestions of the compiler (when it wasn't crashing) and somehow made it work that way 😅
This allows the `FilterParams` struct to be `Send` which helps with migrating this endpoint to async/await.
… implementations
64839b6
to
fb99e47
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10021 +/- ##
==========================================
- Coverage 89.22% 89.19% -0.04%
==========================================
Files 295 295
Lines 30784 30785 +1
==========================================
- Hits 27468 27459 -9
- Misses 3316 3326 +10 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
aka. fighting with the borrow checker and internal compiler errors... 🙈
but hey, it compiles now and seems to work, so I guess that's good? 😆