Skip to content

Conversation

@seanzatzdev
Copy link
Contributor

When attempting to create the rest api spec for the new sampling config APIs, i encountered an error due to the get_all_sample_configuration response's array not being named.

This PR updates the get_all_sample_configuration response to name the returned array in order to conform with elastic API standards.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the get_all_sample_configuration API response format to conform with Elastic API standards by wrapping the array of configurations in a named object property. Previously, the API returned a raw array, but now it returns an object with a "configurations" property containing the array.

  • Updated the response structure to wrap the configuration array in a named "configurations" property
  • Modified all corresponding test assertions to reference the new nested structure

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
GetAllSampleConfigurationAction.java Updated toXContent method to wrap the configurations array in an object with "configurations" property
10_basic.yml Updated all test assertions to reference the new "configurations" property instead of accessing the raw array

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@seanzatzdev seanzatzdev marked this pull request as ready for review October 28, 2025 20:40
@seanzatzdev seanzatzdev added >non-issue :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP labels Oct 28, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label Oct 28, 2025
@seanzatzdev seanzatzdev requested a review from masseyke October 28, 2025 20:40
@seanzatzdev seanzatzdev enabled auto-merge (squash) October 28, 2025 21:01
Copy link
Member

@masseyke masseyke left a comment

Choose a reason for hiding this comment

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

LGTM

@seanzatzdev seanzatzdev merged commit 3304416 into elastic:main Oct 28, 2025
34 checks passed
@seanzatzdev seanzatzdev deleted the update-get-all-response branch October 28, 2025 22:44
masseyke added a commit to masseyke/elasticsearch that referenced this pull request Oct 28, 2025
elasticsearchmachine pushed a commit that referenced this pull request Oct 29, 2025
I failed to merge in main before merging #137271 so I didn't pick up the
changes in #137290. This accounts for them.
chrisparrinello pushed a commit to chrisparrinello/elasticsearch that referenced this pull request Nov 3, 2025
chrisparrinello pushed a commit to chrisparrinello/elasticsearch that referenced this pull request Nov 3, 2025
…7301)

I failed to merge in main before merging elastic#137271 so I didn't pick up the
changes in elastic#137290. This accounts for them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >non-issue Team:Data Management Meta label for data/management team v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants