Skip to content

Add paging support#245

Closed
hxjuneja wants to merge 3 commits into
wikimedia:masterfrom
hxjuneja:paging
Closed

Add paging support#245
hxjuneja wants to merge 3 commits into
wikimedia:masterfrom
hxjuneja:paging

Conversation

@hxjuneja
Copy link
Copy Markdown
Member

@hxjuneja hxjuneja commented May 11, 2015

Note - This pull depends upon wikimedia/restbase-mod-table-cassandra#97

gwicke
gwicke reviewed May 11, 2015
View reviewed changes
Comment thread config.example.yaml Outdated
Copy link
Copy Markdown
Member

@gwicke gwicke May 11, 2015

Choose a reason for hiding this comment

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

saltKey is easy to misunderstand as something connected to salt, the config management system. Perhaps call it just 'salt'?

@gwicke
Copy link
Copy Markdown
Member

gwicke commented May 11, 2015

This needs tests.

@hxjuneja hxjuneja changed the title Add paging support [WIP] Add paging support May 11, 2015
@hxjuneja hxjuneja changed the title [WIP] Add paging support Add paging support May 13, 2015
@coveralls
Copy link
Copy Markdown

coveralls commented May 13, 2015

Coverage Status

Coverage increased (+1.75%) to 86.18% when pulling 3a2f703 on hardikj:paging into 7c35da7 on wikimedia:master.

gwicke
gwicke reviewed May 14, 2015
View reviewed changes
Comment thread config.example.yaml Outdated
Copy link
Copy Markdown
Member

@gwicke gwicke May 14, 2015

Choose a reason for hiding this comment

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

500 or 250 might be more sensible here.

Copy link
Copy Markdown
Member Author

@hxjuneja hxjuneja May 14, 2015

Choose a reason for hiding this comment

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

I used 1000 here because of this limit - https://github.com/wikimedia/restbase/pull/245/files#diff-159808c267853355eee587915c06aae6L317

Right now default_page_size is been used by those end points which make request to cassandra backend, do you want me to use it for other ends points as well?

@gwicke
Copy link
Copy Markdown
Member

gwicke commented May 14, 2015

Looks good to me overall, only a couple of inline notes.

@gwicke
Copy link
Copy Markdown
Member

gwicke commented Jun 2, 2015

This needs a rebase. restbase-mod-table-cassandra 0.6.6 is published.

@hxjuneja hxjuneja force-pushed the paging branch 2 times, most recently from 76a2b0c to 44cafa5 Compare June 3, 2015 01:19
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 3, 2015

Coverage Status

Coverage increased (+1.75%) to 86.18% when pulling 44cafa5 on hardikj:paging into 0c873a8 on wikimedia:master.

1 similar comment
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 3, 2015

Coverage Status

Coverage increased (+1.75%) to 86.18% when pulling 44cafa5 on hardikj:paging into 0c873a8 on wikimedia:master.

@gwicke gwicke closed this Jun 11, 2015
@gwicke
Copy link
Copy Markdown
Member

gwicke commented Jun 11, 2015

Closed in favor of #257, which has some additional cleanup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@gwicke gwicke

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hxjuneja @gwicke @coveralls