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

Restructure JSON output for better server-side pagination support. #91

Open
AlmightyOatmeal opened this issue Jul 6, 2016 · 4 comments

Comments

@AlmightyOatmeal
Copy link

While pagination is enabled by default, there is nothing in the output to indicate the total number of results nor any indication that there are more results. This could be misleading and makes adding a front-end pagination control next to impossible without making additional API calls to check for data or custom methods to count the results.

Instead of returning just an array, I would recommend something along the lines of:

results = { 'num_results': 0, 'objects': [...], 'page': 1, 'total_pages': 1 }

(note: "objects" would be the array containing the actual results instead of just the results themselves being returned)

That provides a total number of results, current page, available pages (total / PG_SIZE); this would be immensely helpful and seems to be a reasonably logical feature to impliment.

@lyschoening
Copy link
Contributor

There is an X-Total-Count header as well as Link headers if I recall correctly. This was inspired from how the GitHub API works. I have been considering an alternative implementation that includes the pagination in the results, but this would be mainly to support additional transfer methods (e.g. WebSocket) that don't have response headers.

@lyschoening
Copy link
Contributor

Also note that both the Python and JavaScript clients handle pagination

@AlmightyOatmeal
Copy link
Author

AlmightyOatmeal commented Jul 6, 2016

@lyschoening: I wasn't aware of the header but that doesn't sound like the most sane location for that type of information but that is my humble opinion. The StackOverflow API may have been a more logical choice to mimic but again that is merely my humble opinion. If I create a JS client, I'm not going to be analyze headers, I've never used an API where such dynamic content is in the headers and not the body of the results.

Being forced to use additional JS or Python clients is a little defeating and will quickly take this project out of the possible solutions to use for a number of upcoming projects. For example, I plan on using this for not only a data resource but also to use as the back-end for a web front-end so parsing headers was never in the project plan; it would be nice to give the user control over where the pagination is to be placed: in the headers or in the response body. I really like how flask-restless lays out their API but they suffer from a number of issues such as traversing relationships necessarily and ghastly performance.

Also in my humble opinion, the StackOverflow, or better yet Twitter, APIs would have been good examples of APIs to model after. Similarly for how flask-restless handles the API layout and their conformity to the JSON API (example: http://jsonapi.org/format/#fetching-pagination).

I would have also liked to see more adherence to the JSON API document structure.

I would have also liked to have seen more documentation or documentation that is correct. I've spotted a number of mistakes, specifically referencing filters or implementations using Python 2.7.x but that's for another day. I would have liked to have seen documentation on how to modify the output structure but the documentation lacks coherent descriptions and usage examples.

As much as I don't like having to specify database relationships when configuring my API endpoints, as it's 2x the administrative overhead, I really do enjoy some of the neat things that flask-potion can do and the performance is fantastic. It's some of the little things that would make me continue my search or sit down and write my own because flask-restless is not meeting the requirements either.

@lyschoening: If you would like to take the conversation out of band, you can reach me on Google Hangouts via jamie (dot) ivanov (at) gmail (dot) com or on Freenode using this same nickname.

@lyschoening
Copy link
Contributor

I am currently collecting ideas for the next major version of Potion, so your comments are very much appreciated. The next version will try to make it easier to add custom implementations and will likely ship with a OAI/Swagger implementation. I don't plan on following the JSON API specification, but you will be able to implement it on top of Potion as you can already do now.

Regarding pagination specifically, if you want to implement the pagination you mention above, in the current version of Potion follow these steps. First make your own subclass of ModelResource:

class YourModelResource(resource.ModelResource):
    @Route.GET('', rel="instances")
    def instances(self, **kwargs):
        return self.manager.paginated_instances(**kwargs)

    instances.request_schema = instances.response_schema = YourInstances()

In this subclass, you can switch out the Instances() class with your own implementation. You can find the class here: https://github.com/biosustain/potion/blob/master/flask_potion/instances.py#L53
You will need to override Instances.format_response() that is inherited from PaginationMixin in the same file and Instances.schema(); specifically response_schema at https://github.com/biosustain/potion/blob/master/flask_potion/instances.py#L131:L131. The code for this part of Potion could be a bit neater and if you have any particular issues please get back to me and I can refactor it with your input.

ModelResource is quite opinionated and designed to work specifically with the clients that have been written for it, but you can customize it to your needs. You can use subclasses to override nearly any detail of a particular ModelResource or manager. This would also allow you to e.g. format individual items like is done in the JSON API spec. (You would need to make your own version FieldSet overriding .schema() and .format() and subclass ResourceMeta to use your own implementation, although we could refactor that for you if you are interested in making these changes).

Please point me to any mistakes you see in the documentation. This can be an issue or even a message on Gitter. PRs would be even better. I don't regularly use Python 2.7 and some other parts of Potion, so it can be difficult to spot these issues.

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

No branches or pull requests

2 participants