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

[APM] Improvements to the ML Settings page #71309

Merged
merged 4 commits into from
Jul 13, 2020
Merged

[APM] Improvements to the ML Settings page #71309

merged 4 commits into from
Jul 13, 2020

Conversation

sorenlouv
Copy link
Member

  • Add loading spinner to "Create Jobs" button (also disable it to avoid double-submission)
  • Run fetch requests in parallel
  • code cleanup

@sorenlouv sorenlouv requested a review from a team as a code owner July 9, 2020 19:52
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Jul 9, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

sorenlouv
sorenlouv commented Jul 9, 2020
View reviewed changes
x-pack/plugins/apm/public/components/app/Settings/anomaly_detection/index.tsx
@@ -80,7 +84,7 @@ export const AnomalyDetection = () => {
) : (
<JobsList
status={status}
anomalyDetectionJobsByEnv={data.jobs}
jobs={data.jobs}
Copy link
Member Author

Choose a reason for hiding this comment

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

No reason to pick a long name when we can pick a short :) (the context makes it obvious what type of job this is)

sorenlouv
sorenlouv commented Jul 9, 2020
View reviewed changes
x-pack/plugins/apm/typings/anomaly_detection.ts
export interface AnomalyDetectionJobByEnv {
environment: string;
job_id: string;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

We can infer the return type from the API via APIReturnType

@sorenlouv sorenlouv added release_note:skip Skip the PR/issue when compiling release notes v7.9.0 labels Jul 9, 2020
@sorenlouv
Copy link
Member Author

@elasticmachine merge upstream

sorenlouv
sorenlouv commented Jul 13, 2020
View reviewed changes
x-pack/plugins/apm/server/routes/settings/anomaly_detection.ts
const [jobs, legacyJobs] = await Promise.all([
getAnomalyDetectionJobs(setup, context.logger),
hasLegacyJobs(setup),
]);
Copy link
Member Author

Choose a reason for hiding this comment

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

Run requests in parallel

cauemarcondes
cauemarcondes approved these changes Jul 13, 2020
View reviewed changes
Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@sorenlouv sorenlouv merged commit 327fed8 into elastic:master Jul 13, 2020
@sorenlouv sorenlouv deleted the ml-followup branch July 13, 2020 15:21
@sorenlouv sorenlouv mentioned this pull request Jul 13, 2020
Merged
sorenlouv added a commit to sorenlouv/kibana that referenced this pull request Jul 13, 2020
@sorenlouv
[APM] Improvements to the ML Settings page (elastic#71309)
0b30fce
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 13, 2020
@gmmorris
Merge branch 'master' into alerting/consumer-based-rbac
cd0522e 
* master: (28 commits)
  skip flaky suite (elastic#71361)
  [Ingest Manager] Add UI to enroll standalone agent (elastic#71288)
  Node options from cfg file for production (elastic#62468)
  [APM] Improvements to the ML Settings page (elastic#71309)
  add old .chromium to gitignore to prevent it from being accidentally committed
  [Ingest Manager] Simplify add/edit package config (integration) form (elastic#71187)
  Ensure Other bucket works on scripted fields. (elastic#71329)
  [APM] Anomaly detection setup link with alert if job doesn't exist (elastic#71229)
  [APM] Anomaly detection integration with transaction duration chart (elastic#71230)
  inclusive language (elastic#71438)
  [Ingest Manager] During fleet setup create an enrollment for every config (elastic#71308)
  Improvements to our developer guide (elastic#67764)
  [SIEM][Detections] Fixes index patterns order (elastic#71270)
  [Metrics + Logs UI] Add test for logs and metrics telemetry (elastic#70858)
  [Maps] Inclusive language (elastic#71427)
  [Logs UI] Unskip log highlight api integration test (elastic#71058)
  [Security_Solution][Resolver] Style adjustments per UX (elastic#71179)
  [Functional test] Increase the timeout to click new vis function (elastic#71226)
  [Discover] Migrate async import of embeddable factory to actual embeddable (elastic#70920)
  fix overflow (elastic#70723)
  ...
sorenlouv added a commit that referenced this pull request Jul 13, 2020
@sorenlouv
[APM] Improvements to the ML Settings page (#71309) (#71462)
4fd8ae9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cauemarcondes cauemarcondes cauemarcondes approved these changes

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v7.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sorenlouv @elasticmachine @kibanamachine @cauemarcondes