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.

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

HLRC: split cluster request converters #33400

Merged
merged 1 commit into from Sep 5, 2018

Conversation

hub-cap
Copy link
Contributor

@hub-cap hub-cap commented Sep 4, 2018

In an effort to encapsulate the different clients, the request
converters are being shuffled around. This splits the ClusterClient
request converters.

In an effort to encapsulate the different clients, the request
converters are being shuffled around. This splits the ClusterClient
request converters.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@@ -134,6 +134,7 @@ protected static void clusterUpdateSettings(Settings persistentSettings,
ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
request.persistentSettings(persistentSettings);
request.transientSettings(transientSettings);
assertOK(client().performRequest(RequestConverters.clusterPutSettings(request)));
assertTrue(execute(
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 moved to using the HLRC here because I did not want the ClusterRequestConverter used in this class.

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 it should be enough to just call the API without any assertion. We already should already throw an exception if the response isn't a 200.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

heh, so then we should remove the bool on the AcknowledgedResponse :)

Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

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

LGTM

@hub-cap hub-cap merged commit 7319bc7 into elastic:master Sep 5, 2018
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 5, 2018
* master:
  Fix deprecated setting specializations (elastic#33412)
  HLRC: split cluster request converters (elastic#33400)
  HLRC: Add ML get influencers API (elastic#33389)
  Add conditional token filter to elasticsearch (elastic#31958)
  Build: Merge xpack checkstyle config into core (elastic#33399)
  Disable IndexRecoveryIT.testRerouteRecovery.
  INGEST: Implement Drop Processor (elastic#32278)
  [ML] Add field stats to log structure finder (elastic#33351)
  Add interval response parameter to AutoDateInterval histogram (elastic#33254)
  MINOR+CORE: Remove Dead Methods ClusterService (elastic#33346)
hub-cap added a commit that referenced this pull request Sep 5, 2018
In an effort to encapsulate the different clients, the request
converters are being shuffled around. This splits the ClusterClient
request converters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants