-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Adding rest actions to get and set data stream settings #127858
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
base: main
Are you sure you want to change the base?
Conversation
Note: the serverless check is failing b/c the yaml rest test uses ILM settings that are not available in serverless. Before merging this PR, I'll exclude those tests from serverless in https://github.com/elastic/elasticsearch-serverless/pull/3890. |
Pinging @elastic/es-data-management (Team:Data Management) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for REST endpoints to get and set data stream settings behind a feature flag.
- Introduces
RestGetDataStreamSettingsAction
andRestUpdateDataStreamSettingsAction
handlers registered when thelogs_stream
flag is enabled - Updates
DataStream.toXContent
to include asettings
field when the feature is on - Adds YAML tests for the new GET/PUT settings APIs and extends existing Java tests to round-trip the settings
Reviewed Changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java | Adds a LOGS_STREAM_FEATURE_FLAG and gates settings output in XContent |
server/src/main/java/org/elasticsearch/action/datastreams/GetDataStreamAction.java | Adds settings ParseField and emits data stream settings when flagged |
server/src/test/java/org/elasticsearch/action/datastreams/GetDataStreamActionTests.java | Imports Settings and tests that settings round-trip through XContent |
modules/data-streams/src/main/java/org/elasticsearch/datastreams/rest/RestGetDataStreamSettingsAction.java | New GET settings handler |
modules/data-streams/src/main/java/org/elasticsearch/datastreams/rest/RestUpdateDataStreamSettingsAction.java | New PUT settings handler |
modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamsPlugin.java | Registers the new handlers behind the feature flag |
modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamFeatures.java | Exposes the logs_stream feature for tests |
modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/240_data_stream_settings.yml | Integration tests for the new settings endpoints |
Files not reviewed (2)
- rest-api-spec/src/main/resources/rest-api-spec/api/indices.get_data_stream_settings.json: Language not supported
- rest-api-spec/src/main/resources/rest-api-spec/api/indices.put_data_stream_settings.json: Language not supported
Comments suppressed due to low confidence (1)
modules/data-streams/src/main/java/org/elasticsearch/datastreams/rest/RestGetDataStreamSettingsAction.java:32
- Typo in action name: should be "get_data_stream_settings_action" for consistency with other handlers.
return "gett_data_stream_settings_action";
@@ -79,6 +80,7 @@ public final class DataStream implements SimpleDiffable<DataStream>, ToXContentO | |||
private static final Logger LOGGER = LogManager.getLogger(DataStream.class); | |||
|
|||
public static final NodeFeature DATA_STREAM_FAILURE_STORE_FEATURE = new NodeFeature("data_stream.failure_store"); | |||
public static final boolean LOGS_STREAM_FEATURE_FLAG = new FeatureFlag("logs_stream").isEnabled(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Directly instantiating a raw FeatureFlag here duplicates the NodeFeature in DataStreamFeatures. Consider centralizing feature checks via the NodeFeature or a shared utility.
public static final boolean LOGS_STREAM_FEATURE_FLAG = new FeatureFlag("logs_stream").isEnabled(); | |
public static final NodeFeature LOGS_STREAM_FEATURE = new NodeFeature("logs_stream"); |
Copilot uses AI. Check for mistakes.
@@ -280,6 +283,10 @@ public List<RestHandler> getRestHandlers( | |||
handlers.add(new RestGetDataStreamOptionsAction()); | |||
handlers.add(new RestPutDataStreamOptionsAction()); | |||
handlers.add(new RestDeleteDataStreamOptionsAction()); | |||
if (DataStream.LOGS_STREAM_FEATURE_FLAG) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Using the static flag on DataStream couples plugin registration to the metadata class. Consider referencing the NodeFeature from DataStreamFeatures for a clearer separation of concerns.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small typo to fix but otherwise, looks good :-)
public class RestGetDataStreamSettingsAction extends BaseRestHandler { | ||
@Override | ||
public String getName() { | ||
return "gett_data_stream_settings_action"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo?
return "gett_data_stream_settings_action"; | |
return "get_data_stream_settings_action"; |
This builds on #127417, #126947, and #127282, adding rest actions to get and set data stream settings. The rest API changes are only visible if the
logs_stream
feature flag is enabled (i.e. you are running a snapshot, or thees.logs_stream_enabled
system property is set to true).Example usage:
Before setting any data stream settings,
returns:
The
settings
field is empty because there are no settings directly on themy-data-stream
data stream. Theeffective_settings
field shows the result of merging this data stream's settings in with the settings it inherited from its template(s).Now we can set settings on the data stream:
and the response is:
The response lets the user know which settings were applied to all backing indices, and which were applied only to the data stream. Now doing a get settings API call shows these new data stream settings:
returns
The get data stream API now also returns the settings that have been applied to the current data stream. Using the example above:
returns:
The
settings
field contains only those settings that have been applied to this data stream, and does not incorporate settings from templates.Note: A dry-run mode will come in a follow-up PR.