Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to new version of EMS #26511

Merged
merged 26 commits into from
Dec 18, 2018

Conversation

thomasneirynck
Copy link
Contributor

@thomasneirynck thomasneirynck commented Dec 3, 2018

DO NOT MERGE.

Closes #26290.


Needs double check

  • can open pre-6.6 saved objects for region maps and coordinate maps (e.g. test with sample data)
  • works with EMS layers and linked layers (e.g. custom TMS and region maps from yml)
  • todo attribution does not work now
  • todo format parsing does not work

Summary

This PR needs to upgrade the EMS client code to match the new upcoming version. It also needs to make the code more reusable, so it can be used in the upcoming GIS-app plugin.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@thomasneirynck thomasneirynck added chore [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation labels Dec 3, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck
Copy link
Contributor Author

@nickpeihl this needs work still (failing tests/some broken UI), but this might already be useful for a first look.

I extracted the ems-client to https://github.com/elastic/kibana/pull/26511/files#diff-600060cb1ee51e89577bdadc0d6014f5 for v6.6.

We can use that module in the GIS app too, both server-side and on the client.

I backfilled serviceSettings (the old EMS-code) so it continues to work (or at least, the test-suite continues to pass). I'll be looking at UI/loose ends next.

Copy link
Member

@nickpeihl nickpeihl left a comment

Choose a reason for hiding this comment

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

So far it looks good. Just a few things I've noticed.

Also the Preview EMS Hotlink link bad, but I can't figure out where that happens.

src/core_plugins/ems_util/common/file_layer.js Outdated Show resolved Hide resolved
}

getHTMLAttribution() {
return this._emsClient.sanitizeMarkdown(this._config.attribution);
Copy link
Member

Choose a reason for hiding this comment

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

There are slight differences in the TMS and Vector file attributions which causes duplicate attributions in the maps. This must be addressed in the upstream tile service rather than in kibana. But I thought I would point it out.

Copy link
Contributor Author

@thomasneirynck thomasneirynck Dec 17, 2018

Choose a reason for hiding this comment

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

thx, yeah, we can look at this. we still need to remove dupes client-side, I'll work on that...

src/core_plugins/ems_util/common/file_layer.js Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck thomasneirynck requested review from a team as code owners December 13, 2018 17:50
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck
Copy link
Contributor Author

@nickpeihl I believe I addressed all outstanding issues.

  • hotlink is not working now, because we need to release corresponding landing page for 6.6 (id of layers have changed). that needs to be a separate PR for the landing page, outside kibana
  • attribution for local layers in the yml, does not work prior to this either, it does not apply markdown formatting. I'll log separete issue for that

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck
Copy link
Contributor Author

jenkins, test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck
Copy link
Contributor Author

jenkins, test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck
Copy link
Contributor Author

jenkins, test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@thomasneirynck
Copy link
Contributor Author

thomasneirynck commented Dec 18, 2018

Backports:
6.x: #27417

thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Dec 18, 2018
This updates Kibana to use the 6.6 version of EMS. It introduces a new library `ems_client`, to parse the manifests. This library will be used by the upcoming GIS-app. The original visualizations continue to use service_settings, but this component has now been rewritten to use this new `ems_client` client.

This backport required manual edits due to differences in the schema.js diff.
thomasneirynck added a commit that referenced this pull request Dec 18, 2018
This updates Kibana to use the 6.6 version of EMS. It introduces a new library `ems_client`, to parse the manifests. This library will be used by the upcoming GIS-app. The original visualizations continue to use service_settings, but this component has now been rewritten to use this new `ems_client` client.

This backport required manual edits due to differences in the schema.js diff.
thomasneirynck added a commit that referenced this pull request Dec 21, 2018
* Update to new version of EMS (#26511)

This updates Kibana to use the 6.6 version of EMS. It introduces a new library `ems_client`, to parse the manifests. This library will be used by the upcoming GIS-app. The original visualizations continue to use service_settings, but this component has now been rewritten to use this new `ems_client` client.

This backport required manual edits due to differences in the schema.js diff.

* [GIS] Add Maps Plugin (#24804)

This adds the MVP of the Phase 1 version of the Maps Plugin to Kibana (#19582).

This is added as a new Stack Feature, requiring a basic license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v6.6.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants