Merge feature/vector tiles branch to master #73872
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is the first iteration that just extracts most obvious shared parts of vector tile and aggregated vector tile. Besides removing code repetition this common basis will simplify dealing with async and other aspects of search api later on.
This change unifies the request parsing and makes it extensible.
Replaces a manual way of building the meta layer with a more generic way of collecting unprocessed elements of the search response.
Adds support for array serialization in meta layer
Adds support for future development under a feature flag
# Conflicts: # x-pack/plugin/spatial/build.gradle # x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java
# Conflicts: # server/src/main/java/org/elasticsearch/common/util/Maps.java
In addition, a few things have been added: * The API now handles cancellation which it turned to be a big performance booster. * The API supports defining a sort field. * RestVectorTileAction has been refactor to one class again. * VectorTileRequest class has been extracted so it can be unit tested.
imotov
added
>non-issue
:Analytics/Geo
elasticmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
jpountz
approved these changes
Jun 15, 2021
There was a problem hiding this comment.
+1 to develop in master with a feature flag rather than using a feature branch. I didn't fully review, only skimmed through the changes, but please don't hold back merging if both @iverase and yourself think that this has good enough quality to be integrated into the master branch.
| flatMap.put(prefix + entry.getKey(), entry.getValue()); | ||
| } | ||
| } | ||
| return flatMap; |
There was a problem hiding this comment.
Do we need to care about the case when a field exists under two different paths in the map, so that one doesn't override the other? Let's document the behavior if we think it's acceptable?
There was a problem hiding this comment.
I think for our purpose overriding is acceptable. We can fix check for it and throw an exception if needed. I will add javadoc with the explanation of the limitations for now.
| throw new IllegalArgumentException( | ||
| "expected es.vector_tile_feature_flag_registered to be unset or [true|false] but was [" + property + "]" | ||
| ); | ||
| } |
There was a problem hiding this comment.
any reason not to use Booleans#parseBoolean?
...ector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryUtils.java
Outdated
Show resolved
Hide resolved
...ctor-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/SimpleFeatureFactory.java
Outdated
Show resolved
Hide resolved
…k/vectortile/feature/FeatureFactoryUtils.java Co-authored-by: Adrien Grand <jpountz@gmail.com>
…k/vectortile/feature/SimpleFeatureFactory.java Co-authored-by: Adrien Grand <jpountz@gmail.com>
|
@elasticmachine update branch |
elasticsearchmachine
pushed a commit
that referenced
this pull request
Aug 5, 2021
* [DOCS] Document `_mvt` API Documents the `_mvt` API endpoint added with #73872. Relates to #75242. * Reword * Rename API * Fix doc.url in JSON spec * Reword * Reword * Add content type to JSON spec * Edits * Fix typo * Reword * Update docs after meeting * Fix typos * Fix `size` default * Updates for #75522 * Fixes * Clean up JSON spec * Fix extent tag * [DOCS] Add `<field>` constraints * Minor clarification * Update for #75697 * Reword * Update for #75621 * Reword default sort * Update for #75367 * Remove unneeded whitespace * Add experimental admon and if flags * [DOCS] Remove ifdefs Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
elasticsearchmachine
pushed a commit
to elasticsearchmachine/elasticsearch
that referenced
this pull request
Aug 5, 2021
* [DOCS] Document `_mvt` API Documents the `_mvt` API endpoint added with elastic#73872. Relates to elastic#75242. * Reword * Rename API * Fix doc.url in JSON spec * Reword * Reword * Add content type to JSON spec * Edits * Fix typo * Reword * Update docs after meeting * Fix typos * Fix `size` default * Updates for elastic#75522 * Fixes * Clean up JSON spec * Fix extent tag * [DOCS] Add `<field>` constraints * Minor clarification * Update for elastic#75697 * Reword * Update for elastic#75621 * Reword default sort * Update for elastic#75367 * Remove unneeded whitespace * Add experimental admon and if flags * [DOCS] Remove ifdefs Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
elasticsearchmachine
added a commit
that referenced
this pull request
Aug 5, 2021
* [DOCS] Document `_mvt` API Documents the `_mvt` API endpoint added with #73872. Relates to #75242. * Reword * Rename API * Fix doc.url in JSON spec * Reword * Reword * Add content type to JSON spec * Edits * Fix typo * Reword * Update docs after meeting * Fix typos * Fix `size` default * Updates for #75522 * Fixes * Clean up JSON spec * Fix extent tag * [DOCS] Add `<field>` constraints * Minor clarification * Update for #75697 * Reword * Update for #75621 * Reword default sort * Update for #75367 * Remove unneeded whitespace * Add experimental admon and if flags * [DOCS] Remove ifdefs Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We would like to continue developing the vector tile feature under the feature flag in master. We realized that the further development requires refactoring the master code and it would be much easier to do if we have everything in the same branch.
Relates #58696