Add backend provider for default settings by bevacqua · Pull Request #6849 · elastic/kibana

bevacqua · 2016-04-11T15:06:22Z

Implementation for #5317

GET /api/kibana/settings/:key retrieves current state for key
POST /api/kibana/settings/:key sets key to posted value
POST /api/kibana/settings sets each key: value in posted changes
DELETE /api/kibana/settings/:key changes key to its default value

In front-end, usage over config service API is preferred.

In server-side, usage over server.uiSettings() is preferred.

…moves settings.

bevacqua · 2016-04-11T19:51:50Z

The current, client-sided implementation of Advanced Settings will send an entire local copy of current settings to Elasticsearch whenever a setting is deleted, using .index(currentSettings). When a setting is just changed, in contrast, it will use .update({ [field]: value }) on the changed field.

I've opted to use the .update approach in all cases, and then when returning the properties from GET /api/kibana/settings, if the field has a value of null it'll use the default setting. Is this okay or would we rather query the current settings and re-index using that, risking running into timing issues?

spalger · 2016-04-11T19:55:14Z

src/plugins/kibana/server/routes/api/settings/register_get.js

+        .then(res => res._source)
+        .then(user => assign(defaults, user, nonEmpty))
+        .then(settings => reply(settings).type('application/json'))
+        .catch(reason => reply(Boom.create(500, `Elasticsearch failure.`)));


Use Boom.wrap() here, since you already have an error

Isn't it bad practice to forward errors as-is to the client-side?

spalger · 2016-04-11T20:00:32Z

src/plugins/kibana/server/routes/api/settings/register_set.js

+    method: 'POST',
+    handler: async function (req, reply) {
+      const key = req.params.key;
+      const value = req.query.value;


I feel like we should require the value key. Otherwise POST /api/kibana/settings/something is actually a DELETE

spalger · 2016-04-11T20:08:06Z

I think that an update with null approach is fine, but keep in mind that the "version" parameter can be used to prevent timing issues.

bevacqua · 2016-04-13T13:40:09Z

Questions I have

Should we update settings on other tabs (via local storage) when they change on the current tab?
Can custom settings be created/used by plugins that I should be aware of? Or what is (Custom setting) in the advanced_row template?

Other than that, and tests, work here is mostly done

spalger · 2016-04-13T14:36:13Z

Should we update settings on other tabs (via local storage) when they change on the current tab?

Lets focus on keeping the behavior we have in place, whatever that is

Can custom settings be created/used by plugins

Not really, but several plugins are hacking new configuration values into the front-end configuration services. Isn't a "blessed" way of doing this a part of this pr?

what is (Custom setting) in the advanced_row template?

This is for the hacks I mentioned earlier. If the configuration value doesn't have a definition in the defaults file it is "custom". See #4924

bevacqua · 2016-04-13T14:37:44Z

Great. I'll make sure to account for that.

…@spalger

…@spalger.

bevacqua · 2016-05-19T18:55:07Z

The latest commits should address all your concerns. 🎉

…preventing mispelling issues.

spalger · 2016-05-19T19:34:43Z

LGTM

bevacqua · 2016-05-19T21:45:04Z

@epixa Wondering if you'd want to take a look at this?

panda01 · 2016-05-20T17:00:33Z

src/plugins/kibana/public/settings/sections/advanced/index.js

    restrict: 'E',
    link: function ($scope) {
-      const configDefaults = Private(ConfigDefaultsProvider);
      const keyCodes = {


This keyCodes doesn't appear to be used

panda01 · 2016-05-20T19:31:49Z

LGTM!

This script will change default timezone used by Kibana to display timestamps. It is ingesting a proper setting directly into '.kibana' index stored in elasticsearch. It will also crate .kibana index if one doesn't exist with default settings replicated from Kibana: 1 shard with 1 replica only. After release of Kibana 5 we will refactor this script to use Kibana API: elastic/kibana#6849

@combor

This script will change the default timezone used by Kibana to display timestamps. It emits a proper setting directly into the `.kibana` index stored in elasticsearch. It will also create a `.kibana` index if one doesn't exist, with the default settings normally used by Kibana: 1 shard with 1 replica only. After the release of Kibana 5 we will refactor this script to use Kibana API: elastic/kibana#6849 @combor & @benhyland

@combor

This script will change the default timezone used by Kibana to display timestamps. It emits a proper setting directly into the `.kibana` index stored in elasticsearch. It will also create a `.kibana` index if one doesn't exist, with the default settings normally used by Kibana: 1 shard with 1 replica only. After the release of Kibana 5 we will refactor this script to use Kibana API: elastic/kibana#6849 @combor & @benhyland

@combor

This script will change the default timezone used by Kibana to display timestamps. It emits a proper setting directly into the `.kibana` index stored in elasticsearch. It will also create a `.kibana` index if one doesn't exist, with the default settings normally used by Kibana: 1 shard with 1 replica only. After the release of Kibana 5 we will refactor this script to use Kibana API: elastic/kibana#6849 @combor & @benhyland

@combor

This script will change the default timezone used by Kibana to display timestamps. It emits a proper setting directly into the `.kibana` index stored in elasticsearch. It will also create a `.kibana` index if one doesn't exist, with the default settings normally used by Kibana: 1 shard with 1 replica only. After the release of Kibana 5 we will refactor this script to use Kibana API: elastic/kibana#6849 Test that the script produces the expected exit codes and hits the expected endpoints. We run the script under test in a separate process since that is closer to reality. This requires us to mock http endpoints using a real http server. We had to upgrade `mimic` and work around a few design issues as the tooling for doing this isn't very good. We might want to reconsider how we do this sort of testing if we have to do a lot of it. @combor & @benhyland

[settings] Add backend provider for default settings.

00f340d

bevacqua self-assigned this Apr 11, 2016

bevacqua added the needs updates label Apr 11, 2016

bevacqua mentioned this pull request Apr 11, 2016

Config service server side #5317

Closed

rashidkpc added the Feature:http label Apr 11, 2016

bevacqua added 2 commits April 11, 2016 12:56

[settings] Implemented GET /api/kibana/settings.

b1d88f5

[settings] Added POST /api/kibana/settings/:key method. Updates or re…

dcfe717

…moves settings.

spalger reviewed Apr 11, 2016
View reviewed changes

[settings] Minor fixes to RESTful settings API.

e30bac7

spalger reviewed Apr 11, 2016
View reviewed changes

bevacqua added 5 commits April 11, 2016 17:28

[settings] DELETE route forked from POST route.

6c47cc6

[settings] Rename registerDelete.

7535867

[settings] Deep restructuring of configuration in front-end.

325c42c

[settings] Properly merge user settings with defaults.

ac3a16f

[settings] Consolidate merge of user and default settings.

1eccd45

bevacqua assigned spalger and unassigned bevacqua Apr 13, 2016

bevacqua added 2 commits April 13, 2016 11:17

[settings] Normalize changeset.

9799cdd

[settings] config.isDefault method, cleanup config code.

243cb30

[settings] Support for custom advanced settings.

de95fdf

bevacqua mentioned this pull request Apr 13, 2016

Replicating advanced settings across browser tabs #6898

Closed

[settings] Readability.

cd054de

bevacqua assigned spalger May 19, 2016

[cleanup] Fix sorting issues in advanced settings view as suggested by …

ef0181f

…@spalger.

bevacqua added 3 commits May 19, 2016 16:08

[guard] Guard clause against unrecognized settings being watched on, …

39f6bd0

…preventing mispelling issues.

[guard] Guard clause on config.get as well.

7aae562

[cleanup] config.get(key) doesn't take fn parameter.

12c4ca4

bevacqua assigned epixa and unassigned spalger May 19, 2016

bevacqua assigned panda01 and unassigned epixa May 20, 2016

panda01 reviewed May 20, 2016
View reviewed changes

[cleanup] Remove unused keyCodes variable.

6222b87

panda01 assigned bevacqua and unassigned panda01 May 20, 2016

bevacqua merged commit 5df51d5 into elastic:master May 20, 2016

bevacqua removed their assignment May 20, 2016

bevacqua mentioned this pull request May 20, 2016

Cleanup index pattern class. #7260

Merged

epixa added the v5.0.0 label May 21, 2016

bevacqua deleted the feature/restify-config branch May 24, 2016 01:53

epixa added the v5.0.0-alpha3 label May 24, 2016

Bargs mentioned this pull request May 26, 2016

[API] Add metafields from uiSettings to index patterns created by ingest API #7301

Merged

dcarley mentioned this pull request Jul 20, 2016

[#121043905] Set kibana timezone to UTC govuk-paas/paas-cf#364

Merged

Conversation

bevacqua commented Apr 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bevacqua commented Apr 11, 2016

Uh oh!

spalger Apr 11, 2016

Choose a reason for hiding this comment

Uh oh!

bevacqua Apr 11, 2016

Choose a reason for hiding this comment

Uh oh!

spalger Apr 11, 2016

Choose a reason for hiding this comment

Uh oh!

spalger commented Apr 11, 2016

Uh oh!

bevacqua commented Apr 13, 2016

Uh oh!

spalger commented Apr 13, 2016

Uh oh!

bevacqua commented Apr 13, 2016

Uh oh!

bevacqua commented May 19, 2016

Uh oh!

spalger commented May 19, 2016

Uh oh!

bevacqua commented May 19, 2016

Uh oh!

panda01 May 20, 2016

Choose a reason for hiding this comment

Uh oh!

bevacqua May 20, 2016

Choose a reason for hiding this comment

Uh oh!

panda01 commented May 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bevacqua commented Apr 11, 2016 •

edited

Loading