Skip to content

Refactor SimpleFeatureFactory so it has no external dependencies#75232

Merged
iverase merged 3 commits intoelastic:masterfrom
iverase:simpleFeatureFactory
Jul 14, 2021
Merged

Refactor SimpleFeatureFactory so it has no external dependencies#75232
iverase merged 3 commits intoelastic:masterfrom
iverase:simpleFeatureFactory

Conversation

@iverase
Copy link
Copy Markdown
Contributor

@iverase iverase commented Jul 12, 2021

SimpleFeatureFactory can generate vector tile features of simple geometries like points and rectangles. This PR refactor that class so it does not have external dependencies, e.g it contains the methods to serialise the feature into a byte array. This makes possible to maybe move this class to server.

This PR adds more test to the feature factory implementation,

@iverase iverase added >non-issue :Analytics/Geo Indexing, search aggregations of geo points and shapes v8.0.0 v7.15.0 labels Jul 12, 2021
@iverase iverase requested a review from imotov July 12, 2021 12:58
@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jul 12, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

Copy link
Copy Markdown
Contributor

@imotov imotov left a comment

Choose a reason for hiding this comment

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

I would be more comfortable if it didn't copy code (even if it is one function) without attribution.

@iverase
Copy link
Copy Markdown
Contributor Author

iverase commented Jul 13, 2021

Right, I think it was a mistake to copy those methods as we just need to write integers using vints. I modified the code so we are suing a BytesStreamOutput to write the features.

Copy link
Copy Markdown
Contributor

@imotov imotov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@iverase iverase merged commit e5eb047 into elastic:master Jul 14, 2021
@iverase iverase deleted the simpleFeatureFactory branch July 14, 2021 05:47
masseyke pushed a commit to masseyke/elasticsearch that referenced this pull request Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Geo Indexing, search aggregations of geo points and shapes >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v7.15.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants