Adding transport actions for getting and updating data stream settings #127417
Conversation
masseyke
added
>non-issue
:Data Management/Data streams
elasticsearchmachine
added
the
Team:Data Management
|
Pinging @elastic/es-data-management (Team:Data Management) |
| CountDownActionListener countDownListener = new CountDownActionListener(dataStreamNames.size() + 1, new ActionListener<>() { | ||
| @Override | ||
| public void onResponse(Void unused) { | ||
| listener.onResponse(new UpdateDataStreamSettingsAction.Response(dataStreamSettingsResponse)); | ||
| } | ||
|
|
||
| @Override | ||
| public void onFailure(Exception e) { | ||
| listener.onFailure(e); | ||
| } | ||
| }); | ||
| countDownListener.onResponse(null); |
There was a problem hiding this comment.
Not so much a PR comment but I'm interested in this pattern here as I haven't seen it before. What's the advantage of this over a bare CountdownLatch?
It feels a little weird to mimic the listener interface but then to call it will a null payload each time but I'm guessing it's something to do with error handling?
There was a problem hiding this comment.
The same thing could definitely have been achieved with a CountdownLatch. With the CountDownActionListener though I can just write my code in the onResponse, and like you said, not worry that I've handled errors correctly, or that I remembered to count down in both the onResonpse and onFailure methods. Not that any of that would have been very hard, but CountDownActionListener was there so I used it.
| ClusterState clusterState | ||
| ) throws Exception { | ||
|
|
||
| ProjectMetadata projectMetadata = clusterState.projectState(updateSettingsTask.projectId).metadata(); |
There was a problem hiding this comment.
No need to convert to ProjectState here
| ProjectMetadata projectMetadata = clusterState.projectState(updateSettingsTask.projectId).metadata(); | |
| ProjectMetadata projectMetadata = clusterState.metadata().getProject(updateSettingsTask.projectId); |
There was a problem hiding this comment.
I hadn't even realized that there were two ways to do it.
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| public class TransportGetDataStreamSettingsAction extends TransportLocalProjectMetadataAction< |
There was a problem hiding this comment.
❤️ for using the local action, thanks!
| @@ -362,6 +407,21 @@ public static ClusterState setRolloverOnWrite( | |||
| ); | |||
| } | |||
|
|
|||
| public void updateSettings( | |||
| ProjectResolver projectResolver, | |||
There was a problem hiding this comment.
Can we pass the project ID directly here instead of the project resolver? That avoids a dependency from this class on the resolver and it's in line with what we're doing in other places (both this class and other classes).
This builds on #126947 and #127282, adding two new transport actions -- one to get the settings of a data stream and one to update the settings of a data stream. The rest action will come in a follow-up PR.