Skip to content
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

created new object that acts as list but also retains metadata #49

Closed
wants to merge 2 commits into from
Closed

created new object that acts as list but also retains metadata #49

wants to merge 2 commits into from

Conversation

hwnjng
Copy link
Contributor

@hwnjng hwnjng commented Jun 19, 2019

In response to: #12.

Object extends list but also takes in metadata => Tackles issue of "I'd rather expect the metadata to be pertinent to the list of objects returned"


This change is Reviewable

@patrickdevivo patrickdevivo requested a review from mmlb June 19, 2019 14:47
@patrickdevivo
Copy link
Contributor

@mmlb - @hwnjng is working on the fmt fixes to make drone happy

@mmlb
Copy link
Contributor

mmlb commented Jun 19, 2019

So I think instead of having the caller deal with pagination somehow we should just handle it in the api. Have a count=-1 default param or something. If its None then fetch all the pages and return everything, otherwise return only count num. Leave per_page unset so api can be the decider or per_page.

This is much more useful to callers by default and leaves them the flexibility choose only some.

@patrickdevivo
Copy link
Contributor

@mmlb totally agree, but we just wanted to keep the scope of this PR small for now just to make the metadata available to begin with, and then figure out how to "automate" the pagination. By API I assume you mean the outward facing interface of this SDK and not the Packet API?

@mmlb
Copy link
Contributor

mmlb commented Jun 19, 2019

Thing is that if we do the automatic fetching then there's no need for this PR.

@mmlb
Copy link
Contributor

mmlb commented Jun 19, 2019

🤔 not unless we want to keep this pr anyway in case the caller calls with a specific count, and we'd have this information for them to re fetch to pull it all in. Not really sure thats a useful case to consider though.

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

Successfully merging this pull request may close these issues.

3 participants