[Snapshot and Restore] Update help text for common repository settings by alisonelizabeth · Pull Request #97652 · elastic/kibana

alisonelizabeth · 2021-04-20T15:03:02Z

The original intent of this PR was to update the help text for some common repository settings to include default values. However, once I started working on it, I noticed the common fields are duplicated across the different repository types. I ended up creating common components for chunk_size, max_restore_bytes_per_sec, and max_snapshot_bytes_per_sec. I also added tests for each repository type, as we currently only had tests for the fs repository.

Note: there are more common settings that can be extracted (e.g., readonly, compressed), but I felt that was out of scope for this PR.

How to test

The CITs should cover all repository types changed in this PR. If you would like to view each one, you will need to install the plugins for s3, gcs, hdfs and azure.

Run yarn es snapshot from kibana dir like normal, then exit out of process
Run cd .es/8.0.0 from kibana dir
Run bin/elasticsearch-plugin install https://snapshots.elastic.co/downloads/elasticsearch-plugins/repository-s3/repository-s3-8.0.0-SNAPSHOT.zip
Replace occurrences of repository-s3 with the plugin you want to install and repeat step 3 for any additional plugins
Run bin/elasticsearch

Screenshots

Release note

The repository form in Snapshot and Restore was updated to include information on default values for common settings.

elasticmachine · 2021-04-21T13:54:23Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

jrodewig

LGTM. I left some non-blocking suggestions. Feel free to ignore those if wanted.

Thanks @alisonelizabeth!

…repository_form/type_settings/common/max_restore.tsx Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

…repository_form/type_settings/common/max_snapshots.tsx Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

…repository_form/type_settings/common/chunk_size.tsx Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

…repository_form/type_settings/common/max_snapshots.tsx Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

…repository_form/type_settings/s3_settings.tsx Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

…repository_form/type_settings/common/max_restore.tsx Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

alisonelizabeth · 2021-04-21T18:17:44Z

@elasticmachine merge upstream

sebelga

Great job @alisonelizabeth ! Tested locally and works as expected 👍

I like to see this composition pattern for our settings, it works very well.

sebelga · 2021-04-23T13:48:16Z


        await act(async () => {
          actions.clickSubmitButton();
-          await nextTick();


Great! So we still have some of those nextTick floating around.. 🤔

Yes, unfortunately. I did not go through all of the tests, so there may be more in SR that need to be addressed.

sebelga · 2021-04-23T13:55:30Z

    text: option,
  }));

+  const updateSettings = (e: React.ChangeEvent<HTMLInputElement>, settingsName: string) => {


nit: when possible I try to keep the function dependencies as small as possible. Probably a habit from testing where it makes mocking easier. 😊 In this case we only really care about the key|value so I would have probably written

const updateSettings = (name: string, value: string) => { ... }

👍 good point. I updated this.

sebelga · 2021-04-23T14:17:01Z

+              example2: <EuiCode>10mb</EuiCode>,
+              example3: <EuiCode>5k</EuiCode>,
+              example4: <EuiCode>1024B</EuiCode>,
+              defaultSize: <EuiCode>40mb</EuiCode>,


I wonder if it wouldn't be better to have a constant object exported with the default sizes. We would then have a single source of truth to update them (and a single file to look at). At the same time there are only 3 values... WDYT?

I considered this as well. I think I'm going to leave as-is for now. The max_snapshots setting has a specific default value, but the other two settings default to unlimited/null.

…pository_help_text

kibanamachine · 2021-04-23T18:42:59Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`snapshotRestore`	176	180	+4

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`snapshotRestore`	470.3KB	459.5KB	-10.8KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`snapshotRestore`	40.8KB	40.6KB	-263.0B

History

💔 Build #122092 failed 189408c
💚 Build #121480 succeeded c9df9af
💔 Build #121445 failed 7380faa
💔 Build #121449 failed e39a78f
💚 Build #121023 succeeded 1459158

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

#97652) (#98199)

update help text for common repository settings

be29c3b

alisonelizabeth added release_note:enhancement v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// Feature:Snapshot and Restore Elasticsearch snapshots and repositories UI labels Apr 20, 2021

update i18n

1459158

alisonelizabeth added the v7.14.0 label Apr 21, 2021

alisonelizabeth marked this pull request as ready for review April 21, 2021 13:54

alisonelizabeth requested a review from a team as a code owner April 21, 2021 13:54

alisonelizabeth requested a review from sebelga April 21, 2021 13:56

jrodewig self-requested a review April 21, 2021 14:33

jrodewig approved these changes Apr 21, 2021

View reviewed changes

alisonelizabeth and others added 6 commits April 21, 2021 12:41

Update x-pack/plugins/snapshot_restore/public/application/components/…

717ca17

…repository_form/type_settings/common/max_restore.tsx Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

Update x-pack/plugins/snapshot_restore/public/application/components/…

7380faa

…repository_form/type_settings/common/max_snapshots.tsx Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

Update x-pack/plugins/snapshot_restore/public/application/components/…

da83a3f

…repository_form/type_settings/common/chunk_size.tsx Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

Update x-pack/plugins/snapshot_restore/public/application/components/…

ccd9388

…repository_form/type_settings/common/max_snapshots.tsx Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

Update x-pack/plugins/snapshot_restore/public/application/components/…

b12e7f2

…repository_form/type_settings/s3_settings.tsx Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

Update x-pack/plugins/snapshot_restore/public/application/components/…

e39a78f

…repository_form/type_settings/common/max_restore.tsx Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

Merge branch 'master' into sr/repository_help_text

c9df9af

sebelga approved these changes Apr 23, 2021

View reviewed changes

alisonelizabeth added 3 commits April 23, 2021 11:57

Merge branch 'master' of https://github.com/elastic/kibana into sr/re…

b8c8476

…pository_help_text

address review feedback

189408c

fix linting error

718b9a4

alisonelizabeth merged commit c75d950 into elastic:master Apr 23, 2021

alisonelizabeth deleted the sr/repository_help_text branch April 23, 2021 18:48

alisonelizabeth mentioned this pull request Apr 23, 2021

[7.x] [Snapshot and Restore] Update help text for common repository settings (#97652) #98199

Merged

alisonelizabeth added a commit that referenced this pull request Apr 23, 2021

[Snapshot and Restore] Update help text for common repository settings (

9d2710b

#97652) (#98199)

Conversation

alisonelizabeth commented Apr 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to test

Screenshots

Release note

Uh oh!

elasticmachine commented Apr 21, 2021

Uh oh!

jrodewig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alisonelizabeth commented Apr 21, 2021

Uh oh!

sebelga left a comment

Choose a reason for hiding this comment

Uh oh!

sebelga Apr 23, 2021

Choose a reason for hiding this comment

Uh oh!

alisonelizabeth Apr 23, 2021

Choose a reason for hiding this comment

Uh oh!

sebelga Apr 23, 2021

Choose a reason for hiding this comment

Uh oh!

alisonelizabeth Apr 23, 2021

Choose a reason for hiding this comment

Uh oh!

sebelga Apr 23, 2021

Choose a reason for hiding this comment

Uh oh!

alisonelizabeth Apr 23, 2021

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Apr 23, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alisonelizabeth commented Apr 20, 2021 •

edited

Loading