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

Rename methods in ChunkedToXContentHelper to be clearer #119342

Merged
merged 3 commits into from
Jan 9, 2025

Conversation

thecoop
Copy link
Member

@thecoop thecoop commented Dec 30, 2024

Rename/consolidate methods in ChunkedToXContentHelper

@thecoop thecoop added :Core/Infra/Core Core issues without another label >refactoring labels Dec 30, 2024
@thecoop thecoop requested a review from a team as a code owner December 30, 2024 10:55
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Dec 30, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Copy link
Contributor

@ldematte ldematte left a comment

Choose a reason for hiding this comment

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

Looks good, just left a couple questions

* @return Singleton iterator for the given item.
*/
public static Iterator<ToXContent> chunk(ToXContent item) {
return Iterators.single(item);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: should you remove singleChunck? It looks like a leftover

Copy link
Member Author

Choose a reason for hiding this comment

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

It's currently used by Serverless, tidying up Serverless uses I'll do separately.

@@ -247,7 +247,7 @@ public Iterator<? extends ToXContent> toXContentChunked(ToXContent.Params params
ChunkedToXContentHelper.field(PARTIAL_FIELD.getPreferredName(), getClusterStateCount(Cluster.Status.PARTIAL)),
ChunkedToXContentHelper.field(FAILED_FIELD.getPreferredName(), getClusterStateCount(Cluster.Status.FAILED)),
// each Cluster object defines its own field object name
ChunkedToXContentHelper.xContentFragmentValuesMapCreateOwnName("details", clusterInfo),
ChunkedToXContentHelper.object("details", clusterInfo.values().iterator()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this equivalent? The implementations are surely different, but maybe the end result is the same. Just checking.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is equivalent - the chunk counts dont change, and the tests still all pass (notice xContentFragmentValuesMapCreateOwnName is just calling getValue.toXContent, ignoring the key entirely)

@thecoop thecoop merged commit 84d04a6 into elastic:main Jan 9, 2025
16 checks passed
@thecoop thecoop deleted the xcontent-helper-methods branch January 9, 2025 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >refactoring Team:Core/Infra Meta label for core/infra team v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants