[SR] Repository list and details UI

[jen-huang]

Adds snapshot repositories list view and details panel. Action buttons (create, delete, edit) in screenshots are just placeholders for now 😄

There is no Redux usage here! React hooks have been very sufficient so far.

Details for a single repository is deep-linked via a path parameter rather than query parameter like in our other apps. This simplifies a lot of code.

Instructions for setting up basic repositories for testing

File system

1. Add the file system path you want to use to elasticsearch.yml or as part of starting up ES:

path:
  repo: /tmp/es-backups

or

yarn es snapshot --license trial -E path.repo=/tmp/es-backups

2. Use Console to add repository, use the file system path above as location setting:

PUT /_snapshot/my_backup
{
  "type": "fs",
  "settings": {
    "location": "/tmp/es-backups",
    "chunk_size": "10mb"
  }
}

3. Adjust settings as necessary, all available settings can be found in docs:
   https://www.elastic.co/guide/en/elasticsearch/reference/master/modules-snapshots.html#_shared_file_system_repository

Readonly

Readonly repositories only take url setting. Documentation: https://www.elastic.co/guide/en/elasticsearch/reference/master/modules-snapshots.html#_read_only_url_repository

It's easy to set up a file: url:

PUT _snapshot/my_readonly_repository
{
  "type": "url",
  "settings": {
    "url": "file:///tmp/es-backups"
  }
}

Source only

Source only repositories are special in that they are basically a wrapper around another repository type. Documentation: https://www.elastic.co/guide/en/elasticsearch/reference/master/modules-snapshots.html#_source_only_repository

This means that the settings that are available depends on the delegate_type parameter. For example, this source only repository delegates to fs (file system) type, so all file system rules and available settings apply:

PUT _snapshot/my_src_only_repository
{
  "type" : "source",
  "settings" : {
    "delegate_type" : "fs",
    "location" : "/tmp/es-backups"
  }
}

Plugin-based repositories:

There are four official repository plugins available: S3, GCS, HDFS, Azure. Please refer to the meta ticket Developer notes section : https://github.com/elastic/elasticsearch-team/issues/118

Available plugin repository settings can be found in docs: https://www.elastic.co/guide/en/elasticsearch/plugins/master/repository.html

No repositories prompt:

List of repositories, with ability to filter by type:

Repository detail panel:

Loading repositories:

Repositories error:

Repository detail panel loading:

Repository detail panel error:

[elasticmachine]

Pinging @elastic/es-ui

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[jen-huang]

VSCode was complaining about missing props on EuiInMemoryTable, it used the definition from here

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[jen-huang]

retest

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[cjcenizal]

🌋 This is really fantastic! I can't believe how little code there is here for all of the functionality delivered. The new hooks syntax and TypeScript sometimes made me feel like I was pushing my brain through a meat grinder, but as I become more and more familiar with it, the more I can appreciate the terseness and elegance that hooks promise. It's soooo much more lightweight than Redux.

I had a few nitty suggestions but nothing major.

@jen-huang Can you provide some steps on how you were testing this? I'd especially love some help with testing the various repo types, so I could see all of the details encoded in the type_details components.

[cjcenizal]

Minor nit: you could throw the error and reverse the order of the conditions to simplify things a bit.

      if (!response.data) {
        throw new Error(response.statusText);
      }
      setData(response.data);

[cjcenizal]

The consumer provides method to this hook so it can also be used to send POST etc requests, and "fetch" seems more geared towards retrieving data (it's also already part of the global namespace). How about renaming this to request and useRequest instead?

[cjcenizal]

Another minor nit, you could use a map instead of a switch to reduce visual noise a bit:

    const map = {
      [FSRepositoryType]: (
        <FormattedMessage
          id="xpack.snapshotRestore.repositoryType.fileSystemTypeName"
          defaultMessage="Shared File System"
        />
      ),

      [ReadonlyRepositoryType]: (
        <FormattedMessage
          id="xpack.snapshotRestore.repositoryType.readonlyTypeName"
          defaultMessage="Read-only URL"
        />
      ),

      [S3RepositoryType]: (
        <FormattedMessage
          id="xpack.snapshotRestore.repositoryType.s3TypeName"
          defaultMessage="AWS S3"
        />
      ),

      /* ... */      
    };

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[jen-huang]

Thanks for the feedback @cjcenizal! I've applied all your suggestions and added a collapsed section to the PR description about setting up repositories. Hope that helps.

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[jen-huang]

retest

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request

[sebelga]

I am not very keen on mixing in the same file constant values and types. Also, the way those are used is still a bit of our old js old patterns, I'll explain 😊

We don't need to declare constants to save guard us from tipos or value update. So we don't need to set a value for FSRepositoryType so we can use it everywhere as a key. When you use them in your switch statement, we have the auto-complete that guides us. And we can only use those values or Typescript will complain. This also means that we don't need all the types imports on top of the file. Take for example documentation_links.ts

Another thing is that I would make use of Enum for the different doc URLs. This (and all) enums should go in the constants.ts file, not in the types as they represent values.

export enum RepositoryDocs {
  fs = 'modules-snapshots.html#_shared_file_system_repository',
  readOnly = 'modules-snapshots.html#_read_only_url_repository',
  ...
}

Then we'd only have 1 import, and the above example (in she screenshot) would end up something like this

The change from const to types would just be

export const FSRepositoryType = 'fs';
export const ReadonlyRepositoryType = 'url';
export const SourceRepositoryType = 'source';
...

// after
export type FSRepository = 'fs'; // no need to add a "Type" at the end, as it can only be used as a type
export type ReadonlyRepository = 'url';
export type SourceRepository = 'source';
...

export type RepositoryType = FSRepository |  ReadonlyRepository | SourceRepository

---

One last thing 😊 (this is more a style guide to agree on), should we create a folder for types and then exposed them all in a barel file?

So this file would be in common/types/repository.ts. And then we'd have an index.ts file exporting all the types. Then when importing types, it would always be from the same path

import { SomeType} from ../common/types

[sebelga]

This will probably be removed because of my previous comment but can we better extract the prop interface for better readability in function component?

interface StateProviderProps {
  reducer: (state: object, action: any) => object;
  initialState: AppStateInterface; // replaced from "object" as you had the type already :)
  children: any;
}

export const StateProvider: React.FunctionComponent<StateProviderProps> = ({
  reducer,
  initialState,
  children,
}) => <AppState.Provider value={useReducer(reducer, initialState)}>{children}</AppState.Provider>;

[sebelga]

I am not sure this is needed unless I am missing something 😊

We can directly export const AppStateProvider = AppState.Provider;

Then in the index.ts it would be

import React, { useReducer } from 'react';
import { AppStateInterface, StateProvider } from './services/app_context';

...

render(
    <I18nContext>
      <HashRouter>
        <AppStateProvider value={useReducer(appStateReducer, appState)}>
          <App />
        </StateProvider>
      </HashRouter>
    </I18nContext>,
    elem
  );

[sebelga]

I don't think it's needed outside this file, I would remove the export.

[sebelga]

It seems strange to me to have a React component exported from our services folder. I think it might we worth to talk about it over zoom with @cjcenizal to decide where it would be best to add our hooks and context providers.

One proposal is to have our code organized by "modules" folder with the following structure. We could even imagine that some of it would be completely reusable across apps.

- modules
  - state // ---> module name
       - index.ts  // everything would be exported form here except the components folder that has its own barel
       - providers.ts (optional)
       - hooks.ts (optional)
       - components (optional)
          - index.ts
       - types.ts // "AppStateInterface" would live here
   - forms
        - index.ts
       - providers.ts (optional)
       ....

[sebelga]

This is how I would have typed the children props:

interface Props {
  children: (fun: deleteRepositoryFunction) => ReactNode;
}

type deleteRepositoryFunction = (names: Array<Repository['name']>) => void;

export const RepositoryDeleteProvider = ({ children }: Props) => {
  const [repositoryNames, setRepositoryNames] = useState<Array<Repository['name']>>([]);
  const [isModalOpen, setIsModalOpen] = useState<boolean>(false);
  const deleteRepository: deleteRepositoryFunction = names => {
    setIsModalOpen(true);
    setRepositoryNames(names);
  };

  ...

[jen-huang]

Thanks for the feedback and great discussion around TS, @sebelga! I've updated the PR with most of your suggestions, except I added another enum for types to be used for conditionals instead of string literals. As we discussed, I'm deferring the folder structure for a later team consensus as we gain more knowledge around using React contexts and hooks.

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request

[sebelga]

Great job @jen-huang with this! It all works very well 😊 I still have a few comments about the way context is used, I will make a follow up when I get back to it tomorrow.

[sebelga]

Like we talked yesterday, this list of constants would be better written with an Enum

export enum RepositoryTypes {
  URL = 'url',
  SOURCE = 'source',
  S3 = 's3',
  ...
}

[jen-huang]

I think you are viewing an outdated file 😄 I moved this to common/types/ and updated with enums: https://github.com/elastic/kibana/pull/33367/files#diff-5c7d6779b8a84817b082813365634361R1

[sebelga]

Yeah sorry! I wrote the comment before you merged it. Then left the review for a while and we got a race condition! 😄

[sebelga]

@jen-huang I created a separate PR to discuss some change proposal. Let's discuss them over there 😊 #33690