[Uptime] Index Status API to Rest by shahzad31 · Pull Request #59657 · elastic/kibana

shahzad31 · 2020-03-09T14:09:28Z

Summary

As part of #58020

Convert Index Status API from GQL to rest !!

elasticmachine · 2020-03-09T14:11:49Z

Pinging @elastic/uptime (Team:uptime)

justinkambic

This looks really good to me, had one comment that we maybe should chat about offline.

justinkambic · 2020-03-10T14:44:16Z

x-pack/legacy/plugins/uptime/common/constants/rest_api.ts

+ */
+
+export enum REST_API_URLS {
+  INDEX_STATUS = '/api/uptime/index_status',


Is the thinking that we will add all the URL paths here as we touch them? If so it might be good to add a follow-up issue for this so to jumpstart adoption.

already done in https://github.com/elastic/kibana/pull/59881/files#diff-a031a8d6fa7f865508620d9c4408ad4d

justinkambic · 2020-03-10T19:15:38Z

x-pack/legacy/plugins/uptime/public/state/api/index_status.ts

+  if (!response.ok) {
+    throw new Error(response.statusText);
+  }
+  const responseData = await response.json();


We keep adding such similar boilerplate for making fetch requests, I think we could really abstract a lot of this code away with a generic function.

also done in https://github.com/elastic/kibana/pull/59881/files

justinkambic · 2020-03-10T19:16:58Z

x-pack/legacy/plugins/uptime/public/state/effects/index_status.ts

+export function* fetchIndexStatusEffect() {
+  yield takeLatest(
+    indexStatusAction.get,
+    fetchEffectFactory(fetchIndexStatus, indexStatusAction.success, indexStatusAction.fail)


fetchEffectFactory(fetchIndexStatus, indexStatusAction.success, indexStatusAction.fail)

I like this get, success, fail pattern.

yes, it really makes usage simple.

justinkambic · 2020-03-10T19:20:49Z

x-pack/plugins/uptime/server/rest_api/index_state/get_index_status.ts

+        },
+      });
+    } catch (e) {
+      return response.internalError({ body: { message: e.message } });


@andrewvc has expressed reservations about returning descriptive error messages, this might be something we want to discuss here.

i am not sure what do you mean by that?

@andrewvc would you have any issue with returning this message? I thought I remembered you expressing concern about security implications of exposing error messages in API responses.

I created elastic/uptime#156 to address this.

justinkambic

LGTM

shahzad31 · 2020-03-16T09:17:39Z

@elasticmachine merge upstream

kibanamachine · 2020-03-16T10:53:36Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 2c2bcac

History

💚 Build #33021 succeeded 15138b8
💚 Build #31815 succeeded 873050c
💔 Build #31765 failed 70fe3dc
💔 Build #31754 failed ae4b32a

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

* master: [Step 1][App Arch] Saved object migrations from kibana plugin to new platform (elastic#59044) adds new test (elastic#60064) [Uptime] Index Status API to Rest (elastic#59657)

* master: (40 commits) skips 'config_open.ts' files from linter check (elastic#60248) [Searchprofiler] Spacing between rendered shards (elastic#60238) Add UiSettings validation & Kibana default route redirection (elastic#59694) [SIEM][CASE] Change configuration button (elastic#60229) [Step 1][App Arch] Saved object migrations from kibana plugin to new platform (elastic#59044) adds new test (elastic#60064) [Uptime] Index Status API to Rest (elastic#59657) [SIEM] Adds 'Closes and opens signals' Cypress test (elastic#59950) [NP] Graph migration (elastic#59409) [ML] Clone analytics job (elastic#59791) Move VALUE_CLICK_TRIGGER and APPLY_FILTER_TRIGGER to ui_action… (elastic#60202) Handle improperly defined Watcher Logging Action text parameter. (elastic#60169) Move select range trigger to uiActions (elastic#60168) [SIEM][CASES] Configure cases: Final (elastic#59358) Closes elastic#59784. Sets xpack.apm.serviceMapEnabled default value true. (elastic#60153) [siem/cypress] update junit filename to be picked up by runbld (elastic#60156) OSS logic added to test environment (elastic#60134) Move canvas setup into app mount (elastic#59926) enable triggers_actions_ui plugin by default (elastic#60137) Expose Elasticsearch from start and deprecate from setup (elastic#59886) ...

kibanamachine · 2020-03-17T11:48:45Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 59657 or prevent reminders by adding the backport:skip label.

kibanamachine · 2020-03-18T11:58:43Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 59657 or prevent reminders by adding the backport:skip label.

* gql to rest * update snap * fix api Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* gql to rest * update snap * fix api Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

gql to rest

ce562e7

andrewvc mentioned this pull request Mar 9, 2020

[Uptime] Convert remaining GQL endpoints to REST #58020

Closed

4 tasks

Merge branch 'master' into index-status-api

ae4b32a

shahzad31 self-assigned this Mar 9, 2020

shahzad31 added release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0 labels Mar 9, 2020

shahzad31 requested a review from justinkambic March 9, 2020 14:11

shahzad31 added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Mar 9, 2020

shahzad31 added 2 commits March 9, 2020 15:36

update snap

70fe3dc

fix api

873050c

justinkambic reviewed Mar 10, 2020

View reviewed changes

shahzad31 added 2 commits March 11, 2020 11:58

Merge branch 'master' into index-status-api

ae422f0

Merge branch 'master' into index-status-api

15138b8

justinkambic approved these changes Mar 13, 2020

View reviewed changes

Merge branch 'master' into index-status-api

2c2bcac

shahzad31 merged commit 168239c into elastic:master Mar 16, 2020

shahzad31 deleted the index-status-api branch March 16, 2020 11:39

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 17, 2020

shahzad31 mentioned this pull request Mar 18, 2020

[7.x] [Uptime] Index Status API to Rest (#59657) #60512

Merged

shahzad31 added a commit to shahzad31/kibana that referenced this pull request Mar 18, 2020

[Uptime] Index Status API to Rest (elastic#59657)

cfe25e9

* gql to rest * update snap * fix api Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 18, 2020

Conversation

shahzad31 commented Mar 9, 2020

Summary

Uh oh!

elasticmachine commented Mar 9, 2020

Uh oh!

justinkambic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinkambic left a comment

Choose a reason for hiding this comment

Uh oh!

shahzad31 commented Mar 16, 2020

Uh oh!

kibanamachine commented Mar 16, 2020

💚 Build Succeeded

History

Uh oh!

kibanamachine commented Mar 17, 2020

Uh oh!

kibanamachine commented Mar 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants