Remove default pagination #1
Conversation
| const page = request.params.page | ||
|
|
||
| page.offset = parseInt(page.offset, 10) || 0 | ||
| page.limit = parseInt(page.limit, 10) || 50 |
There was a problem hiding this comment.
A few questions:
- What does this solve?
- Why not make this configurable rather than unlimited?
- MemoryHandler is not used for many records in production. Are you changing the sequelize-handler as well?
There was a problem hiding this comment.
I have a pr up for https://jira.uncharted.software/browse/DCE-2930, but after speaking with Rob it seems that the issue has more to do with the pagination defaults in the framework.
These defaults were causing us issues on requests not made with query in the client such as findAll, where it was only returning 50 when we were expecting the entire dataset which caused bugs intermittently. Making it configurable here will also only replicate the issue we have on the client where, from what I understand, we've had to periodically increase the limits we've set for pagination. Doing it this way lets the client explicitly specify when/how many to paginate by.
I think only MemoryHandler needs updating, from what I can tell sequelize already has checks in place:
https://github.com/jagql/store-sequelize/blob/master/lib/sqlHandler.js#L524
There was a problem hiding this comment.
This theoretically solves our paging issues. We should be able to get rid of our ember-data store.query override, and the server-side equivalents.
3 participants
Removing default pagination as it has been causing intermittent issues.