Parse user type hints directly to ValuesSourceTypes #65916
Conversation
…f SOLID principles trying to reduce maintenance and modification costs by improving understandability, reliability, reusability and robustness.
…ctices of SOLID principles trying to reduce maintenance and modification costs by improving understandability, reliability, reusability and robustness." This reverts commit 90fb919
…f SOLID principles trying to reduce maintenance and modification costs by improving understandability, reliability, reusability and robustness.
…ctices of SOLID principles trying to reduce maintenance and modification costs by improving understandability, reliability, reusability and robustness." This reverts commit 90fb919
not-napoleon
left a comment
not-napoleon
left a comment
There was a problem hiding this comment.
I appreciate your effort here, but unfortunately we can't just swap one class for the other like this. Changing the wire format is quite tricky, as we need to take into account backwards compatibility and cross cluster search requests. In this case, it's even trickier because I don't want to break existing queries by changing the accepted list of strings for type names.
So, for this to work, it needs to parse and output the same json that we currently accept, and also to serialize and deserialize the same bytes, or at least have a compatibility layer. If you want to take that on, you're welcome to, but please be warned this is not an easy refactor. In general, I wouldn't suggest something like this for a first time contributor (which is why it's not labeled "good first issue").
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; |
There was a problem hiding this comment.
Wildcard imports are forbidden in our style guide. Not sure why that isn't being enforced automatically right now (I'm looking into it), but even so, please leave the imports as is.
See https://github.com/elastic/elasticsearch/blob/master/CONTRIBUTING.md#java-language-formatting-guidelines. I recommend you follow the instructions there to set up your IDE to use our auto-formatting settings.
There was a problem hiding this comment.
Hi Mark, intellij automatically applied wildcard imports, sorry about that.
I just followed the instructions and configured my IDE.
elasticmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
4 participants
Closes #53424. Parses the user's input directly into a ValuesSourceType.