Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Tasks API - Seek/Keyset based pagination #115

Merged
merged 9 commits into from
Jul 7, 2022

Conversation

gmourier
Copy link
Member

@gmourier gmourier commented Feb 21, 2022

🤖 API Diff

Summary

task resource lists now support seek pagination, allowing users to browse multiple sets of task items.

Summary Key Points

Motivation

Following the specification aiming to stabilize the task API resource, we want to give Meilisearch capabilities to adapt to many use-cases, thus involving unpredictable orders of magnitude regarding task growth over time.

This feature should be usable by both solo developers and technical teams.

The objectives set during the discovery's exploration phase for the pagination of the task lists are:

  • Performance should remain nearly constant, regardless of task volume.
  • The results must be browsable in a consistent way between each call to the API, no matter how much the number of tasks is increased or reduced while the navigation is happening.
  • Do not reinvent the wheel and therefore facilitate its implementation by selecting a proven paging interface.
  • A subsidiary, but critical objective, is to feed our API guideline to respond quickly to such needs in the future within the company. By proposing a proven solution, we can build faster with confidence.

@gmourier gmourier added Ready For Review Feature specification must be reviewed. v0.28 labels Feb 21, 2022
@gmourier gmourier changed the title Move cursor based pagination spec to tasks API spec Tasks API - Cursor based pagination Feb 21, 2022
@gmourier gmourier requested a review from MarinPostma March 1, 2022 16:51
@gmourier gmourier force-pushed the add-cursor-pagination-to-tasks-api-spec branch from 5935ff4 to 7625ced Compare March 14, 2022 16:55
@bidoubiwa
Copy link
Contributor

bidoubiwa commented Mar 16, 2022

I exported all my tasks and it weights 3.1M

3.1M Mar 16 12:35 tasks.json
latest uid being "uid":11400

So 💯💯💯

@gmourier gmourier mentioned this pull request Mar 30, 2022
1 task
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

Clean spec @gmourier 🧱 🎉

text/0060-tasks-api.md Outdated Show resolved Hide resolved
@gmourier gmourier force-pushed the add-cursor-pagination-to-tasks-api-spec branch from 7625ced to 3a4a53f Compare May 16, 2022 11:26
@gmourier gmourier added OpenAPI Update OpenAPI specification. Q3:2022 labels May 17, 2022
@github-actions
Copy link

github-actions bot commented May 17, 2022

🚨 Breaking API change detected:

Modified (1)

  • GET /tasks
    • Query parameters added: limit, from
    • Response modified: 200
      • Body attributes added: limit, from, next
      • [Breaking] Body attribute modified: results

View documentation diff

Powered by Bump

@gmourier gmourier changed the title Tasks API - Cursor based pagination Tasks API - Seek/Keyset based pagination May 18, 2022
@gmourier
Copy link
Member Author

Signal @meilisearch/docs-team and @meilisearch/devrel-team; perhaps we should use the terms seek or keyset pagination.

Using the term cursor does not seem to be appropriate. Indeed, pagination by cursor makes me think that a state is established on the server-side to manipulate a cursor's instance, which is not the case here. There is no particular state being created.

@gmourier
Copy link
Member Author

gmourier commented May 24, 2022

@meilisearch/integration-team and @meilisearch/core-team

I have a design question that I'm not sure about. In the specification, I've specified a maximum acceptable value of 100 for the limit field to make sure that the performance stays good.

What do you think about not setting a maximum acceptable value for the limit field and let users report performance problems to adjust this value in the future if needed?

@irevoire
Copy link
Member

Personally, I would prefer not to have any hard limit for that route. If someone has a performance problem while asking for 1000000 things, he’ll know it’s his fault.

Also, that’s what I usually see on other products.

@curquiza
Copy link
Member

Since the /tasks endpoint is a management endpoint and is not exposed to the wild internet (contrary to the /search endpoint), I agree with Tamo, it's ok not putting any limit

@gmourier
Copy link
Member Author

  • Specification uses the word "keyset" instead of "cursor"
  • Removes the boundaries for the limit query-parameter

text/0060-tasks-api.md Outdated Show resolved Hide resolved
text/0060-tasks-api.md Outdated Show resolved Hide resolved
bors bot added a commit to meilisearch/meilisearch that referenced this pull request May 31, 2022
2399: Update the tasks endpoints r=MarinPostma a=Kerollmops

This PR wraps all the changes related to the `tasks` endpoints, it is related to #2377 but doesn't close it. I will create a new PR to work on [the seek-based pagination](meilisearch/specifications#115).

I wanted to do something cool with Github: being able to merge multiple PR in this one, to help review changes one by one, unfortunately, Github doesn't allow creating empty PRs. I also struggled with git itself when it comes to merging things in the right order, so I decided that I would add all of the changes in this single PR. I will list the changes and references to the specs here.

 - [x] Tasks statuses and types must be case insensitive
 - [x] Tasks statuses, types and indexUid must accept the `*` selector
 - [ ] Rename the `TaskDetails` struct fields

## Changes

- [ ] Add seek-based pagination following [the spec](meilisearch/specifications#115) 
- [x] Add filtering on the `/tasks` endpoint following [this spec](meilisearch/specifications#116)
  - [x] Add filtering capabilities on `type`, `status` and `indexUid` for `GET` `task` lists endpoints.
  - [x] It is possible to specify several values for a filter using the `,` character. e.g. `?status=enqueued,processing`
  - [x] Between two different filters, an AND operation is applied. e.g. `?status=enqueued&type=indexCreation` is equivalent to `status=enqueued AND type = indexCreation`
- [x] Remove `GET /indexes/:indexUid/tasks`. It can be replaced by `GET /tasks?indexUid=:indexUid`
- [x] Remove `GET /indexes/:indexUid/tasks/:taskUid`.
- [x] Rename `uid` to `taskUid` in the `202 - Accepted` task response return by every asynchronous tasks (ex: index creation, document addition...)
- [x] Rename some task properties
  - [x] `documentPartial`-> `documentAdditionOrUpdate`
  - [x] `documentAddition`-> `documentAdditionOrUpdate`
  - [x] `clearAll` -> `documentDeletion` 

Co-authored-by: Kerollmops <clement@meilisearch.com>
Copy link
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

I update the behavior we talked about with the integration team and I changed the behavior about out-of-bounds from parameters that you describe but that is more consistent with the API.

text/0060-tasks-api.md Outdated Show resolved Hide resolved
text/0060-tasks-api.md Outdated Show resolved Hide resolved
text/0060-tasks-api.md Outdated Show resolved Hide resolved
text/0060-tasks-api.md Outdated Show resolved Hide resolved
text/0060-tasks-api.md Outdated Show resolved Hide resolved
text/0060-tasks-api.md Outdated Show resolved Hide resolved
text/0060-tasks-api.md Outdated Show resolved Hide resolved
text/0060-tasks-api.md Outdated Show resolved Hide resolved
text/0060-tasks-api.md Outdated Show resolved Hide resolved
gmourier and others added 2 commits June 1, 2022 15:43
@gmourier gmourier force-pushed the add-cursor-pagination-to-tasks-api-spec branch from 5a29808 to eed0178 Compare June 1, 2022 14:07
bors bot added a commit to meilisearch/meilisearch that referenced this pull request Jun 2, 2022
2445: Seek-based tasks list r=ManyTheFish a=Kerollmops

This PR implements the seek-based pagination for the tasks list following [the spec](meilisearch/specifications#115).

Co-authored-by: Kerollmops <clement@meilisearch.com>
bors bot added a commit to meilisearch/meilisearch that referenced this pull request Jun 2, 2022
2445: Seek-based tasks list r=Kerollmops a=Kerollmops

This PR implements the seek-based pagination for the tasks list following [the spec](meilisearch/specifications#115).

Co-authored-by: Kerollmops <clement@meilisearch.com>
Copy link
Contributor

@bidoubiwa bidoubiwa left a comment

Choose a reason for hiding this comment

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

Very nice 🔥

text/0060-tasks-api.md Outdated Show resolved Hide resolved
text/0060-tasks-api.md Show resolved Hide resolved
text/0060-tasks-api.md Show resolved Hide resolved
text/0060-tasks-api.md Outdated Show resolved Hide resolved
gmourier and others added 2 commits June 9, 2022 12:01
Co-authored-by: Tommy <68053732+dichotommy@users.noreply.github.com>
@gmourier gmourier requested review from bidoubiwa and dichotommy June 9, 2022 10:04
@gmourier gmourier merged commit eb097d9 into develop Jul 7, 2022
@gmourier gmourier deleted the add-cursor-pagination-to-tasks-api-spec branch July 7, 2022 09:54
gmourier added a commit that referenced this pull request Jul 7, 2022
* Move cursor based pagination spec to tasks API spec

* remove pagination as a future capability

* Clarify boundaries for limit query parameter

* Update OpenApi specification

* Remove limit field boundaries

* Apply suggestions from code review

Co-authored-by: Clément Renault <renault.cle@gmail.com>

* Update open-api.yml and removes cursor term mentions

* Update text/0060-tasks-api.md

Co-authored-by: Tommy <68053732+dichotommy@users.noreply.github.com>

* Remove  route mention in seek-keyset pagination section

Co-authored-by: Clément Renault <renault.cle@gmail.com>
Co-authored-by: Tommy <68053732+dichotommy@users.noreply.github.com>
gmourier added a commit that referenced this pull request Jul 11, 2022
* Bump open-api.yml to v0.28

* Telemetry - Add `x-meilisearch-client` query parameter (#145)

* Introduce the x-meilisearch-client query parameter

* Update text/0034-telemetry-policies.md

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>

* Update text/0034-telemetry-policies.md

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>

* GeoSearch — Support string type for `_geo` `lat` and `lng` fields (#83)

* Update specification to support string type for _geo lat and lng fields

* mention types mixing for _geo object

* Tasks API - Rename `uid` to `taskUid` in the `202 - Accepted` Summarized Task Response (#144)

* Rename 202 uid to taskUid

* Update text/0060-tasks-api.md

Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com>

Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com>

* Tasks API - Seek/Keyset based pagination (#115)

* Move cursor based pagination spec to tasks API spec

* remove pagination as a future capability

* Clarify boundaries for limit query parameter

* Update OpenApi specification

* Remove limit field boundaries

* Apply suggestions from code review

Co-authored-by: Clément Renault <renault.cle@gmail.com>

* Update open-api.yml and removes cursor term mentions

* Update text/0060-tasks-api.md

Co-authored-by: Tommy <68053732+dichotommy@users.noreply.github.com>

* Remove  route mention in seek-keyset pagination section

Co-authored-by: Clément Renault <renault.cle@gmail.com>
Co-authored-by: Tommy <68053732+dichotommy@users.noreply.github.com>

* Tasks API - Filter tasks list by `type`/`status`/`indexUid` (#116)

* move filtering tasks by status/type parameter to task api spec

* Update specification

* Add details about case-sensitivy + rework error message

* Introducing naming changes plus make the specification a source of truth instead of a changelog

* Remove a future possibility being introduced

* misc - replace createIndex to the right type and add the missing type field to the 202 Response resource

* Dumps API - Make dump creation an asynchronous task (#139)

* wip

* Make a dump creation a visible asynchronous task

* Add precisions

* Update open-api.yml

* Add ommited type field for summarized task response

* Add future possibilities

* Apply suggestions from code review

Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com>

* Precise that indexUid can be null

* Precise priorization of dumpCreation task over other task types

* Keep taskUid for 202 response

* remove dumps.get API key action

Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com>

* Search API - Remove/Rename confusing fields (#135)

* Rename nbHits, remove exhaustive* boolean fields

* Rename approximativeNbHits to estimatedTotalHits

* Update open-api.yaml

* Apply naming changes for facet distribution and showing matches position

* Add a telemetry for facet distribution usage

* API Guideline - Return list of API resources under a `results` array (#138)

* Place list of documents under a results array on /documents

* Add results array for indexes object list

* Add the future of indexes pagination

* Update open-api.yml

* Fix typo

* Apply suggestions from code review

Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com>

* Add offset/limit pagination for indexes and API keys

* Try to add multipe refs to a response object

Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com>

* Remove name field (#140)

* Documents API - `displayedAttributes` should not impact the documents API / Rename `attributesToRetrieve` to `fields` (#143)

* Specifies that displayedAttributes setting does not impact the GET documents endpoint

* Rename attributeToRetrieve to fields on /documents

* Add a future possibily to rejectt a field from a document in the given response

* Precise behavior details about fields query parameter

* Add fields query parameter on GET /indexes/{index}/documents/{docId}

* API Keys - Determinist API Keys + Security changes (#148)

* Add an uid to make API Keys determinists, plus add a non-unique human readable name field to ease reading information

* Describe errors for uid and name fields

* Apply suggestions from code review

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>

* misc: add precisions

* Reorganize route descriptions

* Update error_code when API Key already exists for a given uid

* Apply suggestions from code review

Co-authored-by: Many the fish <legendre.maxime.isn@gmail.com>

* Add new keys actions, remove master-key changes, introduce a new error for immutable field and update tenant token

* Update open-api spec

* Update immutable_field error message

* Apply suggestions from code review

Co-authored-by: Many the fish <legendre.maxime.isn@gmail.com>

* Mention that the Default Admin API Key can manage keys

* Specify that the JWT Tenant Token must be enrypted with the API Key value

* Update the spec regarding the description of the Admin API Key to be up-to-date

* Add uid_or_key url param to update and delete a key

* Update text/0085-api-keys.md

Co-authored-by: Many the fish <legendre.maxime.isn@gmail.com>

* Update text/0085-api-keys.md

Co-authored-by: Many the fish <legendre.maxime.isn@gmail.com>

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
Co-authored-by: Many the fish <legendre.maxime.isn@gmail.com>
Co-authored-by: Kerollmops <clement@meilisearch.com>

* Geosearch - Enhance lat/lng format error messages (#149)

* Update the geosearch error message

* misc: organiser error message in the right specification

Co-authored-by: Guillaume Mourier <guillaume@meilisearch.com>

* Introduces HTTP Verbs changesto be compliant regarding a Rest API (#152)

* Telemetry - Replace `x-meilisearch-client` query parameter by `X-Meilisearch-Client` header (#150)

* Removes x-meilisearch-client, replace it by a header

* Remove capslock

* fix typo (#151)

* Mention telemetry.meilisearch.com (#153)

* update ranking rules error message (#154)

* Misc — Update dump versions compatibility table (#156)

* Update dump table

* Update text/0105-dumps-api.md

* Settings API - Customize the hard limits for `pagination` and `faceting` (#157)

* Introduces specification files

* Update files name

* branch telemetry

* Update open-api.yml

* Update text/0034-telemetry-policies.md

Co-authored-by: Clément Renault <renault.cle@gmail.com>

* update open-api.yml

* Update text/157-faceting-setting-api.md

Co-authored-by: Clément Renault <renault.cle@gmail.com>

* Rename limitedTo to maxTotalHits

* Specify order of returned facet

Co-authored-by: Clément Renault <renault.cle@gmail.com>

* Add dumpCreation task type to OpenAPI.yml

* Tasks filtering params to be in query instead of path on OpenAPI spec

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com>
Co-authored-by: Clément Renault <renault.cle@gmail.com>
Co-authored-by: Tommy <68053732+dichotommy@users.noreply.github.com>
Co-authored-by: Many the fish <legendre.maxime.isn@gmail.com>
Co-authored-by: Kerollmops <clement@meilisearch.com>
Co-authored-by: Tamo <tamo@meilisearch.com>
Co-authored-by: ad hoc <postma.marin@protonmail.com>
Co-authored-by: Clémentine Urquizar - curqui <clementine@meilisearch.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
OpenAPI Update OpenAPI specification. Q3:2022 Ready For Review Feature specification must be reviewed. v0.28
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants