Skip to content
This repository was archived by the owner on Dec 16, 2024. It is now read-only.

Allow paging in Table.scan. #470

Merged
merged 2 commits into from
May 2, 2018
Merged

Allow paging in Table.scan. #470

merged 2 commits into from
May 2, 2018

Conversation

pgavlin
Copy link
Member

@pgavlin pgavlin commented May 1, 2018

Add an overload of Table.scan that accepts a callback that will be
invoked as each page of results is fetched. If this callback returns
false, no more pages will be fetched.

These changes also fix a bug wherein the AWS implementation of
Table.scan would not fetch all results if they had been paginated.

Add an overload of Table.scan that accepts a callback that will be
invoked as each page of results is fetched. If this callback returns
`false`, no more pages will be fetched.

These changes also fix a bug wherein the AWS implementation of
Table.scan would not fetch all results if they had been paginated.
Copy link
Contributor

@lukehoban lukehoban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

aws/table.ts Outdated
}
params.ExclusiveStartKey = result.LastEvaluatedKey;
}
return items;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this unconditionally means the types sort of "lie". We will end up always returning an array - and perhaps surprisingly, an empty array in the case that you passed in a callback.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had not thought about doing this conditionally--I'll give that a shot.

const db = await getDb();
const result = await db.scan({
const params: any = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a shame we can't use a type annotation here, but I think due to the await import it isn't easy to name this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not sure how we'd type it.

@pgavlin pgavlin merged commit eca4b5b into master May 2, 2018
@pgavlin pgavlin deleted the ScanPages branch May 2, 2018 03:25
return items;
} else {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: tihs is effectively equivalent to just "return items";

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants