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.

Sign up for GitHub

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

Jump to bottom

[Maps] Explicitly pass fetch function to ems-client #61846

Merged
merged 3 commits into from
Mar 31, 2020
Merged

[Maps] Explicitly pass fetch function to ems-client #61846

merged 3 commits into from
Mar 31, 2020

Conversation

kindsun
Copy link
Contributor

@kindsun kindsun commented Mar 30, 2020

The EMS Client lib is currently used by both the new Maps server and the legacy Maps client service. The lib allows the option of providing a fetch function. We should be using this option to differentiate between using standard JS fetch for client-side fetch calls vs. node-fetch for server-side calls rather than importing and using node-fetch by default.

We've effectively gotten away with using node-fetch from the client up to this point. In the course of completing #60942, the optimizer produced the following error. The error isn't super-informative, but I traced it back to importing node-fetch in the ems-client:

image

This PR will be followed by another PR to update the ems-client lib to not import node-fetch and instead rely on the passed in fetchFunction

@kindsun kindsun requested a review from a team as a code owner March 30, 2020 16:29
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@kindsun kindsun changed the title Explicitly pass fetch function to ems-client [Maps] Explicitly pass fetch function to ems-client Mar 30, 2020
@kindsun kindsun added the release_note:skip Skip the PR/issue when compiling release notes label Mar 30, 2020
@kindsun kindsun mentioned this pull request Mar 30, 2020
Merged
@kindsun
Copy link
Contributor Author

kindsun commented Mar 30, 2020 •
edited
Loading

Actually this is less simple than it appears. Currently getting the following errors:

image

Resolved. When passing Javascript fetch, it's necessary to wrap the function first.

@kindsun
Copy link
Contributor Author

kindsun commented Mar 30, 2020

retest

nickpeihl
nickpeihl approved these changes Mar 30, 2020
View reviewed changes
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.

lgtm! tested in chrome and code review.

@kindsun
Copy link
Contributor Author

kindsun commented Mar 31, 2020

@elasticmachine merge upstream

@kindsun
Copy link
Contributor Author

kindsun commented Mar 31, 2020

FYI: @nickpeihl Per our conversation earlier, I was able to confirm IE 11 compat with the wrapped fetch call in place

image

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

thomasneirynck
thomasneirynck approved these changes Mar 31, 2020
View reviewed changes
Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

👍

@kindsun kindsun merged commit d73671e into elastic:master Mar 31, 2020
@kindsun kindsun deleted the pass-fetch-function-to-ems-client branch March 31, 2020 14:03
@kindsun kindsun mentioned this pull request Mar 31, 2020
Merged
kindsun pushed a commit to kindsun/kibana that referenced this pull request Mar 31, 2020
@elasticmachine
[Maps] Explicitly pass fetch function to ems-client (elastic#61846)
2012df3 
* Add fetchFunction binding appropriate version of fetch for ems client

* Wrap standard window fetch prior to passing to ems-client

Co-authored-by: Elastic Machine <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 31, 2020
@gmmorris
Merge remote-tracking branch 'upstream/master' into alerting/start-trial
2d80df2 
* upstream/master: (69 commits)
  Adding PagerDuty icon to connectors cards (elastic#60805)
  Fix drag and drop flakiness (elastic#61993)
  Grok debugger migration (elastic#60658)
  Endpoint: Fix resolver SVG position issue (elastic#61886)
  [SIEM] version 7.7 rule import (elastic#61903)
  Added styles to make combobox list items wider for alerting flyout (elastic#61894)
  [UA] Tight worker loop can cause high CPU usage (elastic#60950)
  [ML] DF Analytics results table: use index pattern field format if one exists (elastic#61709)
  [ML] Catching unknown index pattern errors (elastic#61935)
  [Discover] Deangularize and euificate sidebar  (elastic#47559)
  Endpoint: Add ts-node dev dependency (elastic#61884)
  Add an onBlur handler for the kuery bar. Only resubmit when input changes. (elastic#61901)
  [ML] Handle Empty Partition Field Values in Single Metric Viewer (elastic#61649)
  Auto interval on date histogram is getting displayed as timestamp per… (elastic#59171)
  [Maps] Explicitly pass fetch function to ems-client (elastic#61846)
  [SIEM][CASE] Fix aria-labels and translations (elastic#61670)
  [ML] Settings: Increase number of items that can be paged in calendars and filters lists (elastic#61842)
  [EPM] update epm filepath route (elastic#61910)
  APM] Set ignore_above to 1024 for telemetry saved object (elastic#61732)
  [Logs UI] Log stream row rendering (elastic#60773)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 1, 2020
@gmmorris
Merge branch 'master' into event-log/query-support
a0e1dcd 
* master: (64 commits)
  Adding PagerDuty icon to connectors cards (elastic#60805)
  Fix drag and drop flakiness (elastic#61993)
  Grok debugger migration (elastic#60658)
  Endpoint: Fix resolver SVG position issue (elastic#61886)
  [SIEM] version 7.7 rule import (elastic#61903)
  Added styles to make combobox list items wider for alerting flyout (elastic#61894)
  [UA] Tight worker loop can cause high CPU usage (elastic#60950)
  [ML] DF Analytics results table: use index pattern field format if one exists (elastic#61709)
  [ML] Catching unknown index pattern errors (elastic#61935)
  [Discover] Deangularize and euificate sidebar  (elastic#47559)
  Endpoint: Add ts-node dev dependency (elastic#61884)
  Add an onBlur handler for the kuery bar. Only resubmit when input changes. (elastic#61901)
  [ML] Handle Empty Partition Field Values in Single Metric Viewer (elastic#61649)
  Auto interval on date histogram is getting displayed as timestamp per… (elastic#59171)
  [Maps] Explicitly pass fetch function to ems-client (elastic#61846)
  [SIEM][CASE] Fix aria-labels and translations (elastic#61670)
  [ML] Settings: Increase number of items that can be paged in calendars and filters lists (elastic#61842)
  [EPM] update epm filepath route (elastic#61910)
  APM] Set ignore_above to 1024 for telemetry saved object (elastic#61732)
  [Logs UI] Log stream row rendering (elastic#60773)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomasneirynck thomasneirynck thomasneirynck approved these changes

@nickpeihl nickpeihl nickpeihl approved these changes

Assignees
No one assigned
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Feature:Coordinate Map Feature:Region Map release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kindsun @elasticmachine @kibanamachine @nickpeihl @thomasneirynck