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.

Sign up for GitHub

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 #310

Merged
merged 3 commits into from
Jul 6, 2022
Merged

Apply tasks changes #310

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b58a799
Apply tasks changes
alallema Jun 30, 2022
4856d04
Merge branch 'bump-meilisearch-v0.28.0' into tasks_changes
alallema Jul 5, 2022
27e3990
Improve condition and linter
alallema Jul 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .code-samples.meilisearch.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ search_post_1: |-
get_task_1: |-
client.GetTask(1);
get_all_tasks_1: |-
client.GetTasks();
client.GetTasks(nil);
Copy link
Member

Choose a reason for hiding this comment

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

Do you know if this is the best way to handle that? I'm not sure if this will be so "intuitive" to the users.

PS: I know there are no optional args in go, so maybe they have their own way to handle that :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can use variadic parameters but I don't think it's a good way to handle it, even if it works it's made for it.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it could work but not for every case unfortunately

get_task_by_index_1: |-
client.Index("movies").GetTask(1)
get_all_tasks_by_index_1: |-
client.Index("movies").GetTasks()
client.Index("movies").GetTasks(nil)
get_one_key_1: |-
client.GetKey("d0552b41536279a0ad88bd595327b96f01176a60c2243e906c52ac02375f9bc4")
get_keys_1: |-
Expand Down
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ func main() {
os.Exit(1)
}

fmt.Println(task.taskID)
fmt.Println(task.taskUID)
}
```

With the `taskID`, you can check the status (`enqueued`, `processing`, `succeeded` or `failed`) of your documents addition using the [task endpoint](https://docs.meilisearch.com/learn/advanced/asynchronous_operations.html#task-status).
With the `taskUID`, you can check the status (`enqueued`, `processing`, `succeeded` or `failed`) of your documents addition using the [task endpoint](https://docs.meilisearch.com/learn/advanced/asynchronous_operations.html#task-status).

#### Basic Search <!-- omit in toc -->

Expand Down
35 changes: 26 additions & 9 deletions client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net/http"
"strconv"
"strings"
"time"

"github.com/golang-jwt/jwt"
Expand Down Expand Up @@ -51,9 +52,9 @@ type ClientInterface interface {
GetVersion() (resp *Version, err error)
Health() (*Health, error)
IsHealthy() bool
GetTask(taskID int64) (resp *Task, err error)
GetTasks() (resp *ResultTask, err error)
WaitForTask(task *Task, options ...WaitParams) (*Task, error)
GetTask(taskUID int64) (resp *Task, err error)
GetTasks(param *TasksQuery) (resp *TaskResult, err error)
WaitForTask(taskUID int64, options ...WaitParams) (*Task, error)
GenerateTenantToken(searchRules map[string]interface{}, options *TenantTokenOptions) (resp string, err error)
}

Expand Down Expand Up @@ -257,10 +258,10 @@ func (c *Client) GetDumpStatus(dumpUID string) (resp *Dump, err error) {
return resp, nil
}

func (c *Client) GetTask(taskID int64) (resp *Task, err error) {
func (c *Client) GetTask(taskUID int64) (resp *Task, err error) {
resp = &Task{}
req := internalRequest{
endpoint: "/tasks/" + strconv.FormatInt(taskID, 10),
endpoint: "/tasks/" + strconv.FormatInt(taskUID, 10),
method: http.MethodGet,
withRequest: nil,
withResponse: resp,
Expand All @@ -273,16 +274,32 @@ func (c *Client) GetTask(taskID int64) (resp *Task, err error) {
return resp, nil
}

func (c *Client) GetTasks() (resp *ResultTask, err error) {
resp = &ResultTask{}
func (c *Client) GetTasks(param *TasksQuery) (resp *TaskResult, err error) {
resp = &TaskResult{}
req := internalRequest{
endpoint: "/tasks",
method: http.MethodGet,
withRequest: nil,
withResponse: &resp,
withQueryParams: map[string]string{},
acceptedStatusCodes: []int{http.StatusOK},
functionName: "GetTasks",
}
if param != nil && param.Limit != 0 {
alallema marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

I think you should allow the user to pass 0 as the limit, since the Meilisearch engine allows it either ":)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree but for that, I should declare limit as *int which would force the user to initialize his struct in several lines such as:

    limit = 10
    query: &TasksQuery{
        Limit: *limit,
    }

And I think for this specific case when limit = 0 there returns an empty return array so I'm not sure that the inconvenience is worth it

req.withQueryParams["limit"] = strconv.FormatInt(param.Limit, 10)
}
if param != nil && param.From != 0 {
req.withQueryParams["from"] = strconv.FormatInt(param.From, 10)
}
if param != nil && param.Status != "" {
req.withQueryParams["status"] = param.Status
alallema marked this conversation as resolved.
Show resolved Hide resolved
}
if param != nil && param.Type != "" {
req.withQueryParams["type"] = param.Type
Copy link
Member

Choose a reason for hiding this comment

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

I think you can define the map inline, and after that, remove the nulls or empties like this:

map[string]string{
   "type": len(param.IndexUID) != 0 ? strings.Join(param.IndexUID, ",") : nil, 
   "limit": ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You cannot put logic inline in a struct declaration in go or I don't know how

}
if param != nil && len(param.IndexUID) != 0 {
req.withQueryParams["indexUid"] = strings.Join(param.IndexUID, ",")
}
if err := c.executeRequest(req); err != nil {
return nil, err
}
Expand All @@ -295,7 +312,7 @@ func (c *Client) GetTasks() (resp *ResultTask, err error) {
// the TaskStatus.
// If no ctx and interval are provided WaitForTask will check each 50ms the
// status of a task.
func (c *Client) WaitForTask(task *Task, options ...WaitParams) (*Task, error) {
func (c *Client) WaitForTask(taskUID int64, options ...WaitParams) (*Task, error) {
if options == nil {
ctx, cancelFunc := context.WithTimeout(context.Background(), time.Second*5)
defer cancelFunc()
Expand All @@ -310,7 +327,7 @@ func (c *Client) WaitForTask(task *Task, options ...WaitParams) (*Task, error) {
if err := options[0].Context.Err(); err != nil {
return nil, err
}
getTask, err := c.GetTask(task.UID)
getTask, err := c.GetTask(taskUID)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion client_index_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func TestClient_CreateIndex(t *testing.T) {
// Make sure that timestamps are also retrieved
require.NotZero(t, gotResp.EnqueuedAt)

_, err := c.WaitForTask(gotResp)
_, err := c.WaitForTask(gotResp.TaskUID)
require.NoError(t, err)

index, err := c.GetIndex(tt.args.config.Uid)
Expand Down
117 changes: 92 additions & 25 deletions client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ func TestClient_GetTask(t *testing.T) {
type args struct {
UID string
client *Client
taskID int64
taskUID int64
document []docTest
}
tests := []struct {
Expand All @@ -606,9 +606,9 @@ func TestClient_GetTask(t *testing.T) {
{
name: "TestBasicGetTask",
args: args{
UID: "TestBasicGetTask",
client: defaultClient,
taskID: 0,
UID: "TestBasicGetTask",
client: defaultClient,
taskUID: 0,
document: []docTest{
{ID: "123", Name: "Pride and Prejudice"},
},
Expand All @@ -617,9 +617,9 @@ func TestClient_GetTask(t *testing.T) {
{
name: "TestGetTaskWithCustomClient",
args: args{
UID: "TestGetTaskWithCustomClient",
client: customClient,
taskID: 1,
UID: "TestGetTaskWithCustomClient",
client: customClient,
taskUID: 1,
document: []docTest{
{ID: "123", Name: "Pride and Prejudice"},
},
Expand All @@ -628,9 +628,9 @@ func TestClient_GetTask(t *testing.T) {
{
name: "TestGetTask",
args: args{
UID: "TestGetTask",
client: defaultClient,
taskID: 2,
UID: "TestGetTask",
client: defaultClient,
taskUID: 2,
document: []docTest{
{ID: "456", Name: "Le Petit Prince"},
{ID: "1", Name: "Alice In Wonderland"},
Expand All @@ -650,14 +650,14 @@ func TestClient_GetTask(t *testing.T) {
task, err := i.AddDocuments(tt.args.document)
require.NoError(t, err)

_, err = c.WaitForTask(task)
_, err = c.WaitForTask(task.TaskUID)
require.NoError(t, err)

gotResp, err := c.GetTask(task.UID)
gotResp, err := c.GetTask(task.TaskUID)
require.NoError(t, err)
require.NotNil(t, gotResp)
require.NotNil(t, gotResp.Details)
require.GreaterOrEqual(t, gotResp.UID, tt.args.taskID)
require.GreaterOrEqual(t, gotResp.UID, tt.args.taskUID)
require.Equal(t, tt.args.UID, gotResp.IndexUID)
require.Equal(t, TaskStatusSucceeded, gotResp.Status)
require.Equal(t, len(tt.args.document), gotResp.Details.ReceivedDocuments)
Expand All @@ -676,6 +676,7 @@ func TestClient_GetTasks(t *testing.T) {
UID string
client *Client
document []docTest
query *TasksQuery
}
tests := []struct {
name string
Expand All @@ -689,6 +690,7 @@ func TestClient_GetTasks(t *testing.T) {
document: []docTest{
{ID: "123", Name: "Pride and Prejudice"},
},
query: nil,
},
},
{
Expand All @@ -699,6 +701,61 @@ func TestClient_GetTasks(t *testing.T) {
document: []docTest{
{ID: "123", Name: "Pride and Prejudice"},
},
query: nil,
},
},
{
name: "TestGetTasksWithLimit",
args: args{
UID: "indexUID",
client: defaultClient,
document: []docTest{
{ID: "123", Name: "Pride and Prejudice"},
},
query: &TasksQuery{
Limit: 1,
},
},
},
{
name: "TestGetTasksWithLimit",
args: args{
UID: "indexUID",
client: defaultClient,
document: []docTest{
{ID: "123", Name: "Pride and Prejudice"},
},
query: &TasksQuery{
Limit: 1,
},
},
},
{
name: "TestGetTasksWithFrom",
args: args{
UID: "indexUID",
client: defaultClient,
document: []docTest{
{ID: "123", Name: "Pride and Prejudice"},
},
query: &TasksQuery{
From: 0,
},
},
},
{
name: "TestGetTasksWithParameters",
args: args{
UID: "indexUID",
client: defaultClient,
document: []docTest{
{ID: "123", Name: "Pride and Prejudice"},
},
query: &TasksQuery{
Limit: 1,
From: 0,
IndexUID: []string{"indexUID"},
},
},
},
}
Expand All @@ -711,14 +768,24 @@ func TestClient_GetTasks(t *testing.T) {
task, err := i.AddDocuments(tt.args.document)
require.NoError(t, err)

_, err = c.WaitForTask(task)
_, err = c.WaitForTask(task.TaskUID)
require.NoError(t, err)

gotResp, err := i.GetTasks()
gotResp, err := i.GetTasks(tt.args.query)
require.NoError(t, err)
require.NotNil(t, (*gotResp).Results[0].Status)
require.NotZero(t, (*gotResp).Results[0].UID)
require.NotNil(t, (*gotResp).Results[0].Type)
if tt.args.query != nil {
if tt.args.query.Limit != 0 {
require.Equal(t, tt.args.query.Limit, (*gotResp).Limit)
} else {
require.Equal(t, int64(20), (*gotResp).Limit)
}
if tt.args.query.From != 0 {
require.Equal(t, tt.args.query.From, (*gotResp).From)
}
}
})
}
}
Expand All @@ -727,7 +794,7 @@ func TestClient_DefaultWaitForTask(t *testing.T) {
type args struct {
UID string
client *Client
taskID *Task
taskUID *Task
document []docTest
}
tests := []struct {
Expand All @@ -740,7 +807,7 @@ func TestClient_DefaultWaitForTask(t *testing.T) {
args: args{
UID: "TestDefaultWaitForTask",
client: defaultClient,
taskID: &Task{
taskUID: &Task{
UID: 0,
},
document: []docTest{
Expand All @@ -756,7 +823,7 @@ func TestClient_DefaultWaitForTask(t *testing.T) {
args: args{
UID: "TestDefaultWaitForTaskWithCustomClient",
client: customClient,
taskID: &Task{
taskUID: &Task{
UID: 0,
},
document: []docTest{
Expand All @@ -776,7 +843,7 @@ func TestClient_DefaultWaitForTask(t *testing.T) {
task, err := c.Index(tt.args.UID).AddDocuments(tt.args.document)
require.NoError(t, err)

gotTask, err := c.WaitForTask(task)
gotTask, err := c.WaitForTask(task.TaskUID)
require.NoError(t, err)
require.Equal(t, tt.want, gotTask.Status)
})
Expand All @@ -789,7 +856,7 @@ func TestClient_WaitForTaskWithContext(t *testing.T) {
client *Client
interval time.Duration
timeout time.Duration
taskID *Task
taskUID *Task
document []docTest
}
tests := []struct {
Expand All @@ -804,7 +871,7 @@ func TestClient_WaitForTaskWithContext(t *testing.T) {
client: defaultClient,
interval: time.Millisecond * 50,
timeout: time.Second * 5,
taskID: &Task{
taskUID: &Task{
UID: 0,
},
document: []docTest{
Expand All @@ -822,7 +889,7 @@ func TestClient_WaitForTaskWithContext(t *testing.T) {
client: customClient,
interval: time.Millisecond * 50,
timeout: time.Second * 5,
taskID: &Task{
taskUID: &Task{
UID: 0,
},
document: []docTest{
Expand All @@ -840,7 +907,7 @@ func TestClient_WaitForTaskWithContext(t *testing.T) {
client: defaultClient,
interval: time.Millisecond * 10,
timeout: time.Second * 5,
taskID: &Task{
taskUID: &Task{
UID: 1,
},
document: []docTest{
Expand All @@ -858,7 +925,7 @@ func TestClient_WaitForTaskWithContext(t *testing.T) {
client: defaultClient,
interval: time.Millisecond * 50,
timeout: time.Millisecond * 10,
taskID: &Task{
taskUID: &Task{
UID: 1,
},
document: []docTest{
Expand All @@ -881,7 +948,7 @@ func TestClient_WaitForTaskWithContext(t *testing.T) {
ctx, cancelFunc := context.WithTimeout(context.Background(), tt.args.timeout)
defer cancelFunc()

gotTask, err := c.WaitForTask(task, WaitParams{Context: ctx, Interval: tt.args.interval})
gotTask, err := c.WaitForTask(task.TaskUID, WaitParams{Context: ctx, Interval: tt.args.interval})
if tt.args.timeout < tt.args.interval {
require.Error(t, err)
} else {
Expand Down
Loading