Skip to content

Modifying ingest request builders#104636

Merged
masseyke merged 32 commits intoelastic:mainfrom
masseyke:fixing-request-builders
Feb 2, 2024
Merged

Modifying ingest request builders#104636
masseyke merged 32 commits intoelastic:mainfrom
masseyke:fixing-request-builders

Conversation

@masseyke
Copy link
Copy Markdown
Member

@masseyke masseyke commented Jan 23, 2024

This changes all of our ingest-related builders other than BulkRequestBuilder (which was already changed in #104927) to inherit from ActionRequestLazyBuilder (added in #104927) rather than ActionRequestBuilder. This means that the requests will not be created until the builder's request() method is called, making upcoming ref counting work much more feasible.

@masseyke masseyke added >enhancement :Distributed/Ingest Node Execution or management of Ingest Pipelines v8.13.0 labels Jan 23, 2024
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Hi @masseyke, I've created a changelog YAML for you.

@masseyke masseyke changed the title Modifying request builders Modifying ingest request builders Jan 25, 2024
@masseyke masseyke marked this pull request as ready for review January 31, 2024 14:21
@elasticsearchmachine elasticsearchmachine added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Jan 31, 2024
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

}

private UpdateByQueryRequest(SearchRequest search, boolean setDefaults) {
public UpdateByQueryRequest(SearchRequest search, boolean setDefaults) {
Copy link
Copy Markdown
Contributor

@joegallo joegallo Jan 31, 2024

Choose a reason for hiding this comment

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

Nit: can this be less than public?

super.apply(request);
request.id(id);
if (sourceMap != null) {
if (sourceContentType == null) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think these might scan better if you insisted on != null checks throughout. Then for example this top one would become:

if (sourceMap != null && sourceContentType != null) {
    request.source(sourceMap, sourceContentType);
else if (sourceContentType != null) {
    request.source(sourceMap);
}

@masseyke masseyke requested a review from joegallo January 31, 2024 17:16
@masseyke masseyke merged commit 64a790f into elastic:main Feb 2, 2024
@masseyke masseyke deleted the fixing-request-builders branch February 2, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Ingest Node Execution or management of Ingest Pipelines >enhancement Team:Data Management (obsolete) DO NOT USE. This team no longer exists. v8.13.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants