#22 | Add getOrCreateIndex functionality #45

arjunrc143 · 2020-10-27T08:58:58Z

No description provided.

eskombro · 2020-10-27T11:33:14Z

Thanks for your PR. It is important that in the getoOrCreateIndex method we catch the Exception only if the error corresponds to an index that exists already. Otherwise we shouldn't proceed as we do not necessarily know where the problem is coming from. You can check in the MeiliSearch response if the errorCode corresponds to index_already_exists.

This will be updated later when we implement a Custom error handler in #7

Can you please add this check?

eskombro · 2020-10-27T11:34:01Z

Also, please link the issue. You can add a comment in your pr like this

Closes #22

src/main/java/com/meilisearch/sdk/Client.java

Co-authored-by: Samuel Jimenez <[email protected]>

…ists message in exception

arjunrc143 · 2020-10-27T14:12:02Z

Hi @arjunrc143

Thanks for your PR. It is important that in the getoOrCreateIndex method we catch the Exception only if the error corresponds to an index that exists already. Otherwise we shouldn't proceed as we do not necessarily know where the problem is coming from. You can check in the MeiliSearch response if the errorCode corresponds to index_already_exists.

This will be updated later when we implement a Custom error handler in #7

Can you please add this check?

Done

arjunrc143 · 2020-10-27T14:12:32Z

closes #22

eskombro · 2020-10-27T16:37:21Z

closes #22

I think this must be done in the PR description :)

I linked it anyways !

eskombro · 2020-10-27T16:42:49Z

This PR seems good! Just need to add a few integration tests ! For example:

Use getOrCreateIndex() and check that the index is created
Use getOrCreateIndex() with an existing index and check that no Exception is thrown, and that the index is returned
Use getOrCreateIndex() 2 or three times in a row...

…lity

arjunrc143 · 2020-10-28T04:24:13Z

@eskombro I have added the integration tests. Please check.

arjunrc143 · 2020-10-28T04:38:16Z

2 integration tests are failing. Will check that.

This reverts commit 90de15b.

eskombro · 2020-10-29T00:13:09Z

@arjunrc143 I checked what's going on, and the problem is that when you get an index_already_exists error, the SDK is throwing an IOException with the message

Server returned HTTP response code: 400 for URL: http://localhost:7700/indexes

So the check you implemented for the error catching (line 123) doesn't work. This problem is coming from what createIndex() returns in case of failure, and so, not your bad or any problem in your PR at all actually.

I'll fix this since I'm implementing a basic error handler, and let you know so you can rebase your branch and it should work fine!

Glad you added those tests 👍

arjunrc143 · 2020-10-29T04:17:46Z

@eskombro Cool. Let me know once this gets fixed. Thanks!

…MeiliSearchConnectionException (meilisearch#46) Custom Exceptions: MeiliSearchException => MeiliSearchApiException + MeiliSearchConnectionException

…nts_by_id

eskombro

Hey @arjunrc143!

We can already use custom Exceptions, Can you please rebase your branch on master?

I did a suggestion on your code, on how to check the MeiliSearchApiException errorCode in this method which is already tested!

ALSO:

Can you please create a getOrCreateIndex(String uid) method without a primaryKey, which calls getOrCreateIndex(String uid, null) ?

public Index getOrCreateIndex(String uid) throws Exception {
	return getOrCreateIndex(uid, null);
}

primaryKey should be optional!

After this PR will be ready to merge! 🎉

src/main/java/com/meilisearch/sdk/Client.java

…ch.sdk.exception to resolve errors

* Implementation of delete list of documents from index. * Add tests for deleteDocuments , fixed existing errors in test module. * change name of package com.meilisearch.sdk.Exception -> com.meilisearch.sdk.exception to resolve errors

Co-authored-by: Samuel Jimenez <[email protected]>

…ists message in exception

…lity

This reverts commit 90de15b.

Co-authored-by: Samuel Jimenez <[email protected]>

…va into meilisearch-22

arjunrc143 · 2020-11-04T13:44:46Z

Hey @arjunrc143!

We can already use custom Exceptions, Can you please rebase your branch on master?

I did a suggestion on your code, on how to check the MeiliSearchApiException errorCode in this method which is already tested!

ALSO:

Can you please create a getOrCreateIndex(String uid) method without a primaryKey, which calls getOrCreateIndex(String uid, null) ?
public Index getOrCreateIndex(String uid) throws Exception {
	return getOrCreateIndex(uid, null);
}
primaryKey should be optional!

After this PR will be ready to merge! 🎉

@eskombro This is done. Can you please review it?

eskombro

Thanks! 🎉

Chris and others added 3 commits October 23, 2020 03:59

Implementation of delete list of documents from index.

c0a72a0

Add tests for deleteDocuments , fixed existing errors in test module.

7ab7a7f

meilisearch#21 | Add getOrCreateIndex functionality

863b84d

arjunrc143 mentioned this pull request Oct 27, 2020

Add GetOrCreateIndex method #22

Closed

2 tasks

eskombro requested changes Oct 27, 2020

View reviewed changes

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

arjunrc143 and others added 2 commits October 27, 2020 19:33

Update src/main/java/com/meilisearch/sdk/Client.java

616747c

Co-authored-by: Samuel Jimenez <[email protected]>

meilisearch#22 | Get index only if the exception has index_already_ex…

099dad0

…ists message in exception

eskombro linked an issue Oct 27, 2020 that may be closed by this pull request

Add GetOrCreateIndex method #22

Closed

2 tasks

Arjun RC added 2 commits October 28, 2020 09:36

meilisearch#22 | Correct the package of TestData and AbstractIT

90de15b

meilisearch#22 | Add integration tests for getOrCreateIndex functiona…

211471f

…lity

Revert "meilisearch#22 | Correct the package of TestData and AbstractIT"

fc50754

This reverts commit 90de15b.

fixed IllegalStateException.

ddef140

eskombro and others added 2 commits November 4, 2020 01:14

Custom Exceptions: MeiliSearchException => MeiliSearchApiException + …

04f855c

…MeiliSearchConnectionException (meilisearch#46) Custom Exceptions: MeiliSearchException => MeiliSearchApiException + MeiliSearchConnectionException

Merge remote-tracking branch 'upstream/master' into add_delete_docume…

37424c7

…nts_by_id

eskombro requested changes Nov 4, 2020

View reviewed changes

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

Chris and others added 5 commits November 4, 2020 12:42

change name of package com.meilisearch.sdk.Exception -> com.meilisear…

9768950

…ch.sdk.exception to resolve errors

add missing import

c883833

Update src/main/java/com/meilisearch/sdk/Client.java

f3b270e

Co-authored-by: Samuel Jimenez <[email protected]>

meilisearch#21 | Add getOrCreateIndex functionality

4ba02d6

arjunrc143 and others added 8 commits November 4, 2020 18:49

Update src/main/java/com/meilisearch/sdk/Client.java

52f8598

Co-authored-by: Samuel Jimenez <[email protected]>

meilisearch#22 | Get index only if the exception has index_already_ex…

faa21dd

…ists message in exception

meilisearch#22 | Correct the package of TestData and AbstractIT

e01af91

meilisearch#22 | Add integration tests for getOrCreateIndex functiona…

7459368

…lity

Revert "meilisearch#22 | Correct the package of TestData and AbstractIT"

8e52501

This reverts commit 90de15b.

Update src/main/java/com/meilisearch/sdk/Client.java

b5c4880

Co-authored-by: Samuel Jimenez <[email protected]>

meilisearch#22 | Add getOrCreateIndex method without primary key

1a71145

Merge branch 'meilisearch-22' of github.com:arjunrc143/meilisearch-ja…

defa8a8

…va into meilisearch-22

eskombro approved these changes Nov 5, 2020

View reviewed changes

eskombro merged commit f05cb9c into meilisearch:master Nov 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

#22 | Add getOrCreateIndex functionality #45

#22 | Add getOrCreateIndex functionality #45

Uh oh!

arjunrc143 commented Oct 27, 2020

Uh oh!

eskombro commented Oct 27, 2020

Uh oh!

eskombro commented Oct 27, 2020

Uh oh!

Uh oh!

arjunrc143 commented Oct 27, 2020

Uh oh!

arjunrc143 commented Oct 27, 2020

Uh oh!

eskombro commented Oct 27, 2020 •

edited

Loading

Uh oh!

eskombro commented Oct 27, 2020 •

edited

Loading

Uh oh!

arjunrc143 commented Oct 28, 2020

Uh oh!

arjunrc143 commented Oct 28, 2020

Uh oh!

eskombro commented Oct 29, 2020 •

edited

Loading

Uh oh!

arjunrc143 commented Oct 29, 2020

Uh oh!

eskombro left a comment •

edited

Loading

Uh oh!

Uh oh!

arjunrc143 commented Nov 4, 2020

Uh oh!

eskombro left a comment

Uh oh!

Uh oh!

#22 | Add getOrCreateIndex functionality #45

#22 | Add getOrCreateIndex functionality #45

Uh oh!

Conversation

arjunrc143 commented Oct 27, 2020

Uh oh!

eskombro commented Oct 27, 2020

Uh oh!

eskombro commented Oct 27, 2020

Uh oh!

Uh oh!

arjunrc143 commented Oct 27, 2020

Uh oh!

arjunrc143 commented Oct 27, 2020

Uh oh!

eskombro commented Oct 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eskombro commented Oct 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arjunrc143 commented Oct 28, 2020

Uh oh!

arjunrc143 commented Oct 28, 2020

Uh oh!

eskombro commented Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arjunrc143 commented Oct 29, 2020

Uh oh!

eskombro left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arjunrc143 commented Nov 4, 2020

Uh oh!

eskombro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eskombro commented Oct 27, 2020 •

edited

Loading

eskombro commented Oct 27, 2020 •

edited

Loading

eskombro commented Oct 29, 2020 •

edited

Loading

eskombro left a comment •

edited

Loading