Skip to content

Conversation

@javanna
Copy link
Member

@javanna javanna commented Jul 6, 2021

Now that AbstractScriptFieldType and LeafRuntimeField are separate, we can further simplify the creation of runtime fields. Their builder always creates a new instance of the same class (LeafRuntimeField), which can be shared throughout all the builders. The only bit that changes is the MappedFieldType instance, which becomes the only required abstract method in the base builder.

Also, the dynamically created runtime field variant that parses its values from source can be created through a builder which makes sure that we reuse as much code as possible.

Now that AbstractScriptFieldType and LeafRuntimeField are separate, we can further simplify the creation of runtime fields. Their builder always creates a new instance of the same class (LeafRuntimeField), which can be shared throughout all the builders. The only bit that changes is the MappedFieldType instance, which becomes the only required abstract method in the base builder.

Also, the dynamically created runtime field variant that parses its values from source can be created through a builder which makes sure that we reuse as much code as possible.
@javanna javanna added :Search/Search Search-related issues that do not fall into other categories >refactoring v8.0.0 v7.15.0 labels Jul 6, 2021
@javanna javanna requested a review from romseygeek July 6, 2021 19:29
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Jul 6, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

Copy link
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

I had one question, otherwise this looks great.

@javanna javanna added auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels Jul 7, 2021
@elasticsearchmachine elasticsearchmachine merged commit 353f189 into elastic:master Jul 7, 2021
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

To backport manually run:
backport --pr 75009

javanna added a commit to javanna/elasticsearch that referenced this pull request Jul 7, 2021
* Simplify RuntimeField creation

Now that AbstractScriptFieldType and LeafRuntimeField are separate, we can further simplify the creation of runtime fields. Their builder always creates a new instance of the same class (LeafRuntimeField), which can be shared throughout all the builders. The only bit that changes is the MappedFieldType instance, which becomes the only required abstract method in the base builder.

Also, the dynamically created runtime field variant that parses its values from source can be created through a builder which makes sure that we reuse as much code as possible.

* checkstyle

* restore MappedFieldType
javanna added a commit that referenced this pull request Jul 7, 2021
Now that AbstractScriptFieldType and LeafRuntimeField are separate, we can further simplify the creation of runtime fields. Their builder always creates a new instance of the same class (LeafRuntimeField), which can be shared throughout all the builders. The only bit that changes is the MappedFieldType instance, which becomes the only required abstract method in the base builder.

Also, the dynamically created runtime field variant that parses its values from source can be created through a builder which makes sure that we reuse as much code as possible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >refactoring :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v7.15.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants