Refactor nodes stats request builders to match requests #54363
Conversation
williamrandolph
added
>non-issue
:Core/Infra/Plugins
|
Pinging @elastic/es-core-infra (:Core/Infra/Plugins) |
| @@ -95,7 +96,9 @@ public void testFailureInConditionalProcessor() { | |||
| ); | |||
| assertTrue(e.getMessage().contains("this script always fails")); | |||
|
|
|||
| NodesStatsResponse r = client().admin().cluster().prepareNodesStats(internalCluster().getNodeNames()).setIngest(true).get(); | |||
| NodesStatsResponse r = client().admin().cluster().prepareNodesStats(internalCluster().getNodeNames()) | |||
| .addMetric(NodesStatsRequest.Metric.INGEST.metricName()) | |||
There was a problem hiding this comment.
Not that it would save a lot of bytes, but what do you think about adding an override that takes a NodesStatsRequest.Metric?
| .addMetric(NodesStatsRequest.Metric.INGEST.metricName()) | |
| .addMetric(NodesStatsRequest.Metric.INGEST) |
This would be a bit more type-safe at least?
There was a problem hiding this comment.
I'd be tempted use some static imports to abbreviate the code a little, too.
There was a problem hiding this comment.
The NodesStatsRequest.Metric construct is just temporary and will eventually be replaced with a more pluggable solution. That is to say, the addMetric method will need to accept strings that are defined by plugins. Those strings will have to be validated dynamically, possibly by the NodeService.
There was a problem hiding this comment.
I agree on static imports improving readability; I'll make that change.
|
@elasticmachine test this please |
* Remove hard-coded setters from NodesInfoRequestBuilder * Remove hard-coded setters from NodesStatsRequest * Use static imports to reduce clutter
…)" This reverts commit d6d664c.
|
Reverted this because it seemed to cause trouble on a PR: #54601 |
|
Reverted the reversion because the problem turned out to be something different. |
) * Refactor nodes stats request builders to match requests (#54363) * Remove hard-coded setters from NodesInfoRequestBuilder * Remove hard-coded setters from NodesStatsRequest * Use static imports to reduce clutter * Remove uses of old info APIs
In #53637 and #53410, we refactored
NodesInfoRequestandNodesStatsRequestto have a general interface for adding metrics to the request. Here, we refactor the corresponding builders to match this interface. The change has been straightforward, so I'm handlingNodesInfoRequestBuilderandNodesStatsRequestBuilderin a single PR.