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

make IndexPattern class static #69379

Closed
7 tasks done
ppisljar opened this issue Jun 17, 2020 · 5 comments
Closed
7 tasks done

make IndexPattern class static #69379

ppisljar opened this issue Jun 17, 2020 · 5 comments
Labels
Feature:Data Views Data Views code and UI - index patterns before 8.0 Meta

Comments

@ppisljar
Copy link
Member

ppisljar commented Jun 17, 2020

In order to be able to use index patterns inside other plugins without dependency on index patterns runtime contract we should make the IndexPatterns class static. (for example using them inside expressions). We want to use the logic to help working with index patterns field list but we will not be loading, saving or updating the index pattern.

that is possible if all the stateful logic is extracted to the index patterns service:

  • index pattern creation (fetching fields, talking to kibana api)
  • saving (talking to kibana api_
  • loading
  • refreshing field list

[ ] these methods should be removed from index pattern class and moved to index patterns service:

  • init: move to indexPatternsService.get(indexPatternId: string)
  • popularizeField: remove, only used by discover, move it to use save, as save will be refactored not to show toasts but rather throw in case of errors, discover can decide to handle that as it wants
  • create: move to indexPatternsService.save(indexPattern: IndexPattern)
  • save: move to indexPatternsService.save(indexPattern: IndexPattern)
  • fetchFields: move to indexPatternsService.updateFields(indexPattern: IndexPattern)
  • refreshFields: remove, move the logic of showing the error message to management app (if updateFields failed)
  • destroy: move to indexPatternService.delete(indexPatternId: string)

[x] remove all toast notifications or passing in error handlers but rather throw when something goes wrong

  • inside index_pattern.ts
  • inside field.ts
  • inside index_patterns.ts

at this point IndexPattern class should not need any dependencies passed in and can be exported statically.

@ppisljar ppisljar added Meta Feature:Data Views Data Views code and UI - index patterns before 8.0 Team:AppArch labels Jun 17, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@mattkime
Copy link
Contributor

@ppisljar Will we need to address dependencies on the FieldFormats registry? Or are those okay?

@mattkime
Copy link
Contributor

We want to use the logic to help working with index patterns field list but we will not be loading, saving or updating the index pattern.

Perhaps it would be helpful to list out the methods you'd use so they could be abstracted into a class that would be inherited by the current IndexPatterns class.

@lukeelmers
Copy link
Member

Reposing my comment from the index patterns serialization PR.

TLDR I think as part of this issue we should also move the initFromSpec operations into the IndexPattern constructor, if it hasn't already been done so in that other PR.

I wonder if the IndexPatterns constructor should be taking in these spec values as optional params, and if they are provided the constructor would handle any necessary processing? IMHO the purpose of the constructor is to perform any init operations... and I think the only reason there's a separate init method currently is because it's making a call to ES via the SO client, which will hopefully be separated out in #69379

At that point it would feel especially odd to have a dedicated initFromSpec, so I'd propose moving this into the constructor either here or at that time

@mattkime
Copy link
Contributor

Notes on implementing this -

init should be simplified before being moved. a number of functions are either called only via init or in one other place. The division of responsibilities could be improved.

init calls

  • prepBody
  • updateFromElasticSearch
    • indexFields
      • isFieldRefreshRequired (can stay on indexPattern)
      • refreshFields
        • _fetchFields

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Data Views Data Views code and UI - index patterns before 8.0 Meta
Projects
None yet
Development

No branches or pull requests

5 participants