Implement proper pagination for list-payments cli #84
Conversation
|
👋 Thanks for assigning @tankyleo as a reviewer! |
| if payments.len() >= target_count as usize || response.next_page_token.is_none() { | ||
| next_page_token = response.next_page_token; | ||
|
|
||
| if payments.len() >= target_count as usize || next_page_token.is_none() { |
There was a problem hiding this comment.
The point of surfacing the next_page_token back to the user is so that they can continue iterating after we've already satisfied their request to provide at least n payments ?
benthecarman
force-pushed
the
pagination-payments
branch
from
2faead0 to
7614706
Compare
benthecarman
force-pushed
the
pagination-payments
branch
from
7614706 to
bfc6312
Compare
benthecarman
force-pushed
the
pagination-payments
branch
from
bfc6312 to
f44c7fc
Compare
|
🔔 1st Reminder Hey @tankyleo! This PR has been waiting for your review. |
| impl From<ListPaymentsResponse> for CliListPaymentsResponse { | ||
| fn from(response: ListPaymentsResponse) -> Self { | ||
| let next_page_token = response.next_page_token.map(format_page_token); | ||
|
|
||
| CliListPaymentsResponse { payments: response.payments, next_page_token } | ||
| } | ||
| } | ||
|
|
||
| fn format_page_token(token: PageToken) -> String { | ||
| format!("{}:{}", token.token, token.index) | ||
| } |
There was a problem hiding this comment.
Is this a better fit in the fn handle_list_payments_request in ldk-server ? Pretty much delete the PageToken type in the proto definition, and switch it to optional string next_page_token = 2.
Like this people who interact with the server programmatically also no longer can mishandle these two pieces ?
Although I agree they get a single object of type PageToken, so may be good enough there.
3 participants
Another thing in the backend but not the cli