-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat(java): waitForTask APIC-478 #521
Conversation
✅ Deploy Preview for api-clients-automation canceled.
|
✗ The generated branch has been deleted.If the PR has been merged, you can check the generated code on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking greatooo
clients/algoliasearch-client-javascript/packages/client-common/src/createRetryablePromise.ts
Outdated
Show resolved
Hide resolved
@@ -216,5 +216,19 @@ public class {{classname}} extends ApiClient { | |||
} | |||
{{/optionalParams.0}} | |||
{{/operation}} | |||
|
|||
{{#isSearchClient}} | |||
public void waitForTask(String indexName, Long taskID, RequestOptions requestOptions) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we allow option overriding like we did for JS? This way people don't have to re-implement the function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep I can add a variant
import java.util.function.Predicate; | ||
import java.util.function.Supplier; | ||
|
||
public class TaskUtils { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could maybe add the defaults at the class level? So we don't have to dig if we want to change it somedays
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did the same as for js, do you want me to change it there too ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep a constant at the top of the file maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry you mean putting the default in the search client or just at the top of this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the top of this file, the default value options (same for JS maybe?)
...lgoliasearch-client-java-2/algoliasearch-core/src/main/java/com/algolia/utils/TaskUtils.java
Outdated
Show resolved
Hide resolved
...lgoliasearch-client-java-2/algoliasearch-core/src/main/java/com/algolia/utils/TaskUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really clean!!
The generation said nope ahah |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
purrfect now!
🧭 What and Why
🎟 JIRA Ticket: APIC-478
Add the
waitForTask
util for the search client, and make it generic if we want to use for other task.I modified the maxTrials and timeout based on test, it can up a lot more that 1 seconds to delete a single object.
🧪 Test
There is no test for the waitTask yet, I will integrate with the incoming client test, but it's tested in the playground.