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] New Platform server shim: update job audit messages routes #57925

Conversation

alvarezmelissa87
Copy link
Contributor

Summary

Related meta issue: #49743

Updates all job audit messages routes to use new platform router

Checklist

Delete any items that are not applicable to this PR.

@alvarezmelissa87 alvarezmelissa87 requested a review from a team as a code owner February 18, 2020 21:48
@alvarezmelissa87 alvarezmelissa87 self-assigned this Feb 18, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

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, just added a suggestion about optional function parameters.

Comment on lines 12 to 13
getJobAuditMessages: (jobId: string | undefined, from: string | undefined) => any;
getAuditMessagesSummary: (jobIds: string[] | undefined) => any;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Could make use of optional parameters like (jobId?: string, from?: string) and (jobIds?: string[]).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

context.ml!.mlClient.callAsCurrentUser
);
const { jobId } = request.params;
const from = request.query.from;
Copy link
Member

Choose a reason for hiding this comment

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

absolute nit, could be const { from } = request.query;
it stands out be cause if the line above it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alvarezmelissa87 alvarezmelissa87 force-pushed the ml-np-job-audit-message-routes branch from f1eb404 to dcb7a1d Compare February 19, 2020 16:23
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

  • 💚 Build #27496 succeeded f1eb404ae7487bd26e45f6defc0f12623046a623

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

@alvarezmelissa87 alvarezmelissa87 merged commit c2d98a0 into elastic:master Feb 19, 2020
alvarezmelissa87 added a commit to alvarezmelissa87/kibana that referenced this pull request Feb 19, 2020
…tic#57925)

* migrate all job audit messages routes to NP

* update types and add missing api names from validation
@alvarezmelissa87 alvarezmelissa87 deleted the ml-np-job-audit-message-routes branch February 19, 2020 18:56
alvarezmelissa87 added a commit that referenced this pull request Feb 19, 2020
…) (#58041)

* migrate all job audit messages routes to NP

* update types and add missing api names from validation
mbondyra added a commit to mbondyra/kibana that referenced this pull request Feb 20, 2020
* master: (136 commits)
  [Visualize] Remove legacy appState in visualize (elastic#57330)
  Use static time for tsvb rollup test (elastic#57701)
  [SIEM] Fix ResizeObserver polyfill (elastic#58046)
  [SIEM][Detection Engine] Fixes return codes where some were rule_id instead of id
  skip flaky suite (elastic#56816)
  skip flaky suite (elastic#58059)
  skip flaky suite (elastic#45348)
  migrates notification server routes to NP (elastic#57906)
  Moved all of the show/hide toggles outside of ordered lists. (elastic#57163)
  [APM] NP Migration - Moves plugin server files out of legacy (elastic#57532)
  [Maps][Telemetry] Migrate Maps telemetry to NP (elastic#55055)
  Embeddable add panel examples (elastic#57319)
  Fix useRequest to support query change (elastic#57723)
  Allow custom paths in plugin generator (elastic#57766)
  [SIEM][Case] Merge header components (elastic#57816)
  [ML] New Platform server shim: update job audit messages routes (elastic#57925)
  [kbn/optimizer] emit success event from reducer when all bundles cached (elastic#57945)
  [APM] Don’t include UI filters when fetching a specific transaction (elastic#57934)
  Upgrade yargs (elastic#57720)
  skip flaky suite (elastic#57762) (elastic#57997) (elastic#57998)
  ...

# Conflicts:
#	src/plugins/advanced_settings/public/management_app/components/field/__snapshots__/field.test.tsx.snap
#	src/plugins/advanced_settings/public/management_app/components/field/field.tsx
#	x-pack/plugins/translations/translations/ja-JP.json
#	x-pack/plugins/translations/translations/zh-CN.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:NP Migration :ml release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants