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

Merge the pagination extension #1327

Closed
duncandewhurst opened this issue Jul 8, 2021 · 5 comments · Fixed by #1422
Closed

Merge the pagination extension #1327

duncandewhurst opened this issue Jul 8, 2021 · 5 comments · Fixed by #1422
Assignees
Labels
Focus - Packages Relating to release packages and record packages
Milestone

Comments

@duncandewhurst
Copy link
Contributor

Creating a new issue for this comment from #928 since the main topic of that issue will be addressed separately:

We should also look into merging in https://github.com/open-contracting-extensions/ocds_pagination_extension.

Originally posted by @jpmckinney in #928 (comment)

@duncandewhurst duncandewhurst added this to the 1.2.0 milestone Jul 8, 2021
@duncandewhurst duncandewhurst added Schema: Fields Relating to adding or deprecating fields in the JSON Schema Focus - Packages Relating to release packages and record packages labels Jul 8, 2021
@duncandewhurst
Copy link
Contributor Author

@jpmckinney - I'm assuming we want to treat the following content from the extension's readme as non-normative guidance and move it to https://standard.open-contracting.org/staging/1.2-dev/en/guidance/build/hosting/. Is there anything else to discuss before preparing a PR?

To construct the next and/or prev URLs, it is encouraged to use query string parameters like:

  • since=TIMESTAMP, to return a page of results that are modified after the since timestamp, in chronological order
  • offset=NUMBER, to return a page of results that are positioned after the offset number, in sequential order (for example, if the results are retrieved from a SQL database)

It is discouraged to use page=NUMBER with results ordered in reverse chronology, because:

  • A given page won't return the same results over time. page=1 will return different results today, next week, and next year.
  • Users can receive duplicate results while paginating. For example, if a new release is published to page 1 while users are paginating, then the result at the bottom of each page will be moved to the top of the following page.
  • It is harder for users to synchronize with the API. With since or offset, users can retrieve new results by submitting the timestamp or offset of their last request. With page, users need to determine which results are new or old.

Reference: HTML link types, 18F API Standards, Government of Canada Standards on APIs, Government of Ontario API Guidelines, OpenActive Realtime Paged Data Exchange.

@duncandewhurst duncandewhurst self-assigned this Aug 23, 2021
@jpmckinney
Copy link
Member

Since the fields are new and optional, we can make normative statements about them.

However, if we are reducing the role of packages (#920), it might not make sense to merge an extension that adds fields to packages. There are many ways to paginate without next/prev links in the data (e.g. GitHub uses HTTP headers, to separate API metadata from data/content): https://docs.github.com/en/rest/guides/traversing-with-pagination

So, I think this issue requires more discussion on whether to go ahead or not.

@jpmckinney jpmckinney added discussion and removed Schema: Fields Relating to adding or deprecating fields in the JSON Schema discussion labels Aug 23, 2021
@duncandewhurst
Copy link
Contributor Author

Hmm. I thought that in #1084 we settled on keeping packages as the 'API format', for which pagination is relevant. That said, I think it's fine to keep pagination as an extension.

Regarding different approaches to pagination, it's easier for aggregators or other users that want to work with OCDS data from multiple publishers if OCDS APIs use a consistent approach. In that respect, it's useful to recommend an approach to encourage standardisation. Recommending an approach also gives a sensible 'default' for implementers who might not know the pros and cons of different options. Of course, if implementers have a preference/use case/reason for using a different approach, they can.

I don't have a particular preference for one approach over another. If I remember correctly, we decided on the links approach based on desk research and community calls.

@kindly might have a view on this.

@jpmckinney
Copy link
Member

Links are certainly the easiest to implement and use, since some users don't even know about HTTP headers.

Since we need to keep packages for 1.x anyway, we might as well add pagination as implemented in the extension.

Rather than a guidance page, add a page for APIs under the Reference section (maybe "API responses").

As part of #1084, we can move the release/record package schema pages under that, to keep the Reference section somewhat organized.

@jpmckinney
Copy link
Member

Rather than a guidance page, add a page for APIs under the Reference section (maybe "API responses").

Noting that since pagination content was added to the guidance for #1095, we might as well just link to it there, were it appears in context without other content that is more purely guidance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Focus - Packages Relating to release packages and record packages
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants