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

[ML] Ensure Anomaly Explorer and Single View do not hang by fixing TypeError: finally is not a function #25961

Merged

Conversation

alvarezmelissa87
Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 commented Nov 20, 2018

Summary

Replaces calls to finally which is not supported (as we move away from Angular) with catch and then to fix TypeError: finally not a function error and prevent Loading... hang due to this error.

image

  • Replaces .finally call in explorer_controller.js
  • Replaces .finally call in timeseriesexplorer_controller.js

Checklist

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

For maintainers

- [ ] This was checked for breaking API changes and was labeled appropriately
- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui

@alvarezmelissa87 alvarezmelissa87 changed the title [ML] Fix TypeError: finally is not a function [ML] Ensure Anomaly Explorer and Single View do not hang by fixing TypeError: finally is not a function Nov 20, 2018
Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@@ -264,7 +264,8 @@ module.controller('MlExplorerController', function (

// Populate the map of jobs / detectors / field formatters for the selected IDs.
mlFieldFormatService.populateFormats(selectedIds, getIndexPatterns())
.finally(() => {
.catch((err) => { console.log('Error setting selected jobs:', err); })
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change this error message to say Error populating field formats. The jobs will be selected ok, it's just actual and typical values will be formatted using the default numeric formatting rather than e.g. as bytes, if a formatter has been applied for that field.

.finally(() => {
// Load the data - if the FieldFormats failed to populate
// the default formatting will be used for metric values.
.catch((err) => { console.log('Error loading job:', err); })
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, this error should say Error populating field formats

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

💔 Build Failed

@peteharverson peteharverson added the non-issue Indicates to automation that a pull request should not appear in the release notes label Nov 21, 2018
@alvarezmelissa87
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alvarezmelissa87 alvarezmelissa87 merged commit 9ea7720 into elastic:master Nov 21, 2018
alvarezmelissa87 added a commit to alvarezmelissa87/kibana that referenced this pull request Nov 21, 2018
…peError: finally is not a function (elastic#25961)

* Replace finally with catch/then to fix typeError

* update error messages
@alvarezmelissa87 alvarezmelissa87 deleted the ml-refactor-finally-to-then branch November 21, 2018 19:29
alvarezmelissa87 added a commit that referenced this pull request Nov 21, 2018
…peError: finally is not a function (#25961) (#26041)

* Replace finally with catch/then to fix typeError

* update error messages
@sophiec20 sophiec20 added the Feature:Anomaly Detection ML anomaly detection label Jun 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Anomaly Detection ML anomaly detection Feature:ml-results legacy - do not use :ml non-issue Indicates to automation that a pull request should not appear in the release notes review v6.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants