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

Apply tasks changes #510

Merged
merged 3 commits into from
Dec 27, 2022
Merged

Apply tasks changes #510

merged 3 commits into from
Dec 27, 2022

Conversation

alallema
Copy link
Contributor

@alallema alallema commented Dec 21, 2022

Pull Request

Related to: meilisearch/integration-guides#205

What does this PR do?

Breaking because enforces the users to use Meilisearch v0.28.0

Changes

  • Remove GET /indexes/:indexUid/tasks. Use GET /tasks?indexUid=:indexUid instead.
  • Remove GET /indexes/:indexUid/tasks/:taskUid. Use GET /tasks/:taskUid instead.
  • Possibility to filter the results:
    • In the endpoint GET /tasks we can filter by: type, status and indexUid.
    • We need to join the array values when filter by status like this: enqueued,processing.
  • index.getTask now use GET /tasks/:task_uid
  • index.getTasks now use GET/tasks?indexUid=movies
  • Rename task uid field to taskUid.
  • index.getTasks accept pagination metadata, added limit (default: 20), from.
  • client.getTasks accept pagination metadata, added limit (default: 20), from.
  • Add TaskInfo class to handle all response from asynchronous operation
  • Create TasksQuery class to handle pagination filtering
  • Create TasksResults class to retrieve responses from getTasks

src/main/java/com/meilisearch/sdk/TasksHandler.java Outdated Show resolved Hide resolved
}

/**
* Retrieves tasks from the client
* Retrieves all TasksHandler at the specified index uid
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if I got this... 👀

Copy link
Contributor Author

@alallema alallema Dec 27, 2022

Choose a reason for hiding this comment

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

I modify it's better?

Copy link
Member

Choose a reason for hiding this comment

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

What I found strange was the TasksHandler part. Maybe you want to Retrieves all tasks at the specified index uid

src/main/java/com/meilisearch/sdk/TasksHandler.java Outdated Show resolved Hide resolved
* @throws MeilisearchException if client request causes an error
*/
TasksResults getTasks(String indexUid, TasksQuery param) throws MeilisearchException {
String[] newIndexUid = new String[param.getIndexUid().length + 1];
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why you had to do this. Can you explain it to me, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's for this specific case:

    TasksQuery query = new TasksQuery().setIndexes(new String[] {"books", "cities"});
    client.index("movies").getTasks();

In case the getTasks() method is called through the IndexesHandler the request of this should be:

/tasks?indexUids=movies,books,cities

* @throws MeilisearchException if the client request causes an error
*/
Task addDocuments(String uid, String document, String primaryKey) throws MeilisearchException {
TaskInfo addDocuments(String uid, String document, String primaryKey)
throws MeilisearchException {
String urlQuery = "/indexes/" + uid + "/documents";
Copy link
Member

Choose a reason for hiding this comment

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

You can apply the URLBuilder here :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this one will be changed during PR about documents changes

src/test/java/com/meilisearch/integration/ClientTest.java Outdated Show resolved Hide resolved
@brunoocasali brunoocasali added the breaking-change The related changes are breaking for the users label Dec 21, 2022
@alallema alallema force-pushed the tasks_changes branch 2 times, most recently from 11ce64d to d177640 Compare December 27, 2022 14:06
@alallema alallema merged commit 9abd107 into result-rename Dec 27, 2022
@alallema alallema deleted the tasks_changes branch December 27, 2022 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change The related changes are breaking for the users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants