Make RestHighLevelClient's Request class public #26627
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Request class is currently package protected, making it difficult for the users to extend the RestHighLevelClient and to use its protected methods to execute requests. This commit makes the Request class public and changes few methods of RestHighLevelClient to be protected. closes elastic#26455
nik9000
approved these changes
Sep 13, 2017
javanna
reviewed
Sep 14, 2017
| @@ -77,20 +79,42 @@ | |||
|
|
|||
| public class RequestTests extends ESTestCase { | |||
|
|
|||
| public void testConstructor() throws Exception { | |||
There was a problem hiding this comment.
can we also test the visibility of the class and its constructor by checking their modifiers?
|
Thanks all for your reviews! @javanna I just added tests for what your requested. Can you please have another look? |
javanna
reviewed
Sep 18, 2017
| @@ -17,19 +17,17 @@ | |||
| * under the License. | |||
| */ | |||
|
|
|||
| package org.elasticsearch.client; | |||
| package org.elasticsearch.client.extension; | |||
There was a problem hiding this comment.
sorry I didn't notice this before, but why changing the package here? It would be nice to be able to test everything we need with reflection, rather than changing package... unless there's another reason why this was done.
There was a problem hiding this comment.
No other reason, just to make it more obvious that CustomRestClient extends the high level rest client while being located in a different package.
There was a problem hiding this comment.
Please let's not make an arbitrary (and inconsistent) change like that. We do not use "extension" packages anywhere else.
|
@javanna I updated the code and remove the |
javanna
approved these changes
Sep 19, 2017
|
elasticsearch-ci retest this please |
tlrx
added a commit
that referenced
this pull request
Sep 20, 2017
Request class is currently package protected, making it difficult for the users to extend the RestHighLevelClient and to use its protected methods to execute requests. This commit makes the Request class public and changes few methods of RestHighLevelClient to be protected.
tlrx
added a commit
that referenced
this pull request
Sep 20, 2017
Request class is currently package protected, making it difficult for the users to extend the RestHighLevelClient and to use its protected methods to execute requests. This commit makes the Request class public and changes few methods of RestHighLevelClient to be protected.
tlrx
added a commit
that referenced
this pull request
Sep 20, 2017
Request class is currently package protected, making it difficult for the users to extend the RestHighLevelClient and to use its protected methods to execute requests. This commit makes the Request class public and changes few methods of RestHighLevelClient to be protected.
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
Sep 20, 2017
* master: [DOCS] Added index-shared4 and index-shared5.asciidoc BulkProcessor flush runnable preserves the thread context from creation time (elastic#26718) Catch exceptions and inform handler in RemoteClusterConnection#collectNodes (elastic#26725) [Docs] Fix name of character filter in example. (elastic#26724) Remove parse field deprecations in query builders (elastic#26711) elastic#26720: Set the correct bwc version after backport to 6.0 Remove deprecated type and slop field in MatchQueryBuilder (elastic#26720) Refactoring of Gateway*** classes (elastic#26706) Make RestHighLevelClient's Request class public (elastic#26627) Deguice ActionFilter (elastic#26691) aggs: Allow aggregation sorting via nested aggregation. Build: Set bwc builds to always set snapshot (elastic#26704) File Discovery: Remove fallback with zen discovery (elastic#26667)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requestclass is currently package protected, making it difficult forthe users to extend the
RestHighLevelClientand to use its protectedmethods to execute requests. This commit makes the
Requestclass publicand changes few methods of
RestHighLevelClientto be protected.closes #26455