Polling mechanism, getBuildStatus #1
Conversation
| while (System.currentTimeMillis() - startTime < timeout) { | ||
| RemoteBuildStatusResponse remoteBuildStatusResponse = getBuildStatus(remoteBuildResponse.getJobId()); | ||
| String taskStatus = remoteBuildStatusResponse.getTaskStatus(); | ||
| switch (taskStatus) { | ||
| case COMPLETED_INDEX_BUILD: | ||
| return remoteBuildStatusResponse; | ||
| case FAILED_INDEX_BUILD: | ||
| String errorMessage = remoteBuildStatusResponse.getErrorMessage(); | ||
| if (errorMessage != null) { | ||
| throw new InterruptedException("Index build failed: " + errorMessage); | ||
| } | ||
| throw new InterruptedException("Index build failed without an error message."); | ||
| case RUNNING_INDEX_BUILD: | ||
| Thread.sleep(pollInterval); | ||
| } | ||
| } |
There was a problem hiding this comment.
this logic should be outside of the RemoteIndexHTTPClient.
There was a problem hiding this comment.
+1, I think the polling implementation should be done in either RemoteIndexBuildStrategy or even as a separate poller class implementation.
There was a problem hiding this comment.
Should the client interface method be getStatus then?
| String taskStatus = remoteBuildStatusResponse.getTaskStatus(); | ||
| switch (taskStatus) { | ||
| case COMPLETED_INDEX_BUILD: | ||
| return remoteBuildStatusResponse; |
There was a problem hiding this comment.
I think we should do some basic path validation somewhere
There was a problem hiding this comment.
Maybe this should happen in readFromRepository? Since this may be different per repo?
There was a problem hiding this comment.
From the repository perspective this is already well-defined. The Repository takes a BlobPath object to return a BlobContainer, and then the BlobContainer takes a file name in readBlob, and none of this is repository specific. I'm more worried about this path being different per remote index build service.
Currently readFromRepository assumes that the input path is a / delimited filepath, but that would ultimately depend on whatever this status response is returning. I think ideally readFromRepository is not even responsible for converting from a String path to a BlobPath + String fileName.
I don't think it would be a bad idea to perform some basic output validation from within the client itself (this would be sort of analogous to how the S3 SDK can be configured to do all the checksum validations for you)
41852e4 to
6a8b40b
Compare
53d8b7a to
1dc54cd
Compare
c651104 to
fd3002f
Compare
6db364c to
75b1afa
Compare
Add RemoteIndexClient initial implementation, its accompanying dependencies, and Build Request, Retry Strategy, and test files Signed-off-by: owenhalpert <ohalpert@gmail.com> # Conflicts: # CHANGELOG.md # src/main/java/org/opensearch/knn/index/KNNSettings.java # src/main/java/org/opensearch/knn/index/codec/nativeindex/NativeIndexBuildStrategyFactory.java # src/main/java/org/opensearch/knn/index/codec/nativeindex/remote/RemoteIndexBuildStrategy.java # src/test/java/org/opensearch/knn/index/codec/nativeindex/remote/RemoteIndexBuildStrategyTests.java # Conflicts: # CHANGELOG.md # src/main/java/org/opensearch/knn/index/codec/nativeindex/remote/RemoteIndexBuildStrategy.java # Conflicts: # CHANGELOG.md # build.gradle # src/main/java/org/opensearch/knn/index/KNNSettings.java # src/main/java/org/opensearch/knn/index/codec/nativeindex/remote/RemoteIndexBuildStrategy.java # src/main/java/org/opensearch/knn/index/remote/RemoteBuildRequest.java # src/main/java/org/opensearch/knn/index/remote/RemoteIndexClient.java
Signed-off-by: owenhalpert <ohalpert@gmail.com>
16df7e9 to
ece9f7b
Compare
ece9f7b to
75ae2bd
Compare
Description
This PR is a followup to 2560. This PR adds the polling mechanism for the client to poll the server.
This PR does not include:
which will go in the next PR.
Related Issues
opensearch-project#2560
opensearch-project#2391
Check List
- [ ] New functionality has been documented.- [ ] API changes companion pull request created.--signoff.- [ ] Public documentation issue/PR created.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.