Simplify EsqlQueryResponse #129031
Simplify EsqlQueryResponse #129031
Conversation
idegtiarenko
added
>refactoring
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
| public Profile(List<DriverProfile> drivers) { | ||
| this.drivers = drivers; | ||
| } | ||
| public record Profile(List<DriverProfile> drivers) implements Writeable { |
There was a problem hiding this comment.
I wonder if we even need that? May be it is worth in-lining List instead?
There was a problem hiding this comment.
I believe I wrote it as a record because I expected we'd add other List<PlanProfile> and List<SomeOtherThingProfile> to it. Mostly, I thought it'd have plan profile in it.
nik9000
left a comment
nik9000
left a comment
There was a problem hiding this comment.
This is helpful, though I think it'd be even more readable to do it the old school way.
| public Profile(List<DriverProfile> drivers) { | ||
| this.drivers = drivers; | ||
| } | ||
| public record Profile(List<DriverProfile> drivers) implements Writeable { |
There was a problem hiding this comment.
I believe I wrote it as a record because I expected we'd add other List<PlanProfile> and List<SomeOtherThingProfile> to it. Mostly, I thought it'd have plan profile in it.
| Iterator<ToXContent> executionInfoRender = executionInfo != null && executionInfo.hasMetadataToReport() | ||
| ? ChunkedToXContentHelper.field("_clusters", executionInfo, params) | ||
| : Collections.emptyIterator(); | ||
| return Iterators.concat( |
There was a problem hiding this comment.
I think this'd be more readable as a traditional
List<ToXContent> all = new ArrayList<>();
addAsyncProperties(all);
all.add(took)
...
I think so.
But that could be because I'm old. |
3 participants
This change tries to simplify EsqlQueryResponse:
conditionalChunkedXContentand migrate all usages to it.List<DriverProfile>. Here I am converting it to record.