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

Clean up TSVB API interface, convert to typescript #57342

Closed
2 of 8 tasks
flash1293 opened this issue Feb 11, 2020 · 2 comments
Closed
2 of 8 tasks

Clean up TSVB API interface, convert to typescript #57342

flash1293 opened this issue Feb 11, 2020 · 2 comments
Labels
Feature:TSVB TSVB (Time Series Visual Builder) impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture

Comments

@flash1293
Copy link
Contributor

flash1293 commented Feb 11, 2020

The TSVB plugin is largely untyped. This is especially problematic because large config objects are sent from the client to the server without a defined shape. A preliminary Joi schema validating the config objects is currently in place in src/plugins/vis_type_timeseries/server/routes/post_vis_schema.ts, but might contain subtle mistakes, especially since saved objects can contain configurations created with old, potentially buggy versions of TSVB.

Additionally the TSVB server side code still assumes the shape of the legacy platform request object in some places - this abstraction is leaking out into the search strategy extension point used by rollup.

To clean this up and get to a well-defined API interface (both for the route and extension point), multiple clean up steps are required:

  • Turn Joi schema into @kbn/schema - this will provide a type we can use elsewhere
  • Type client code to make sure it conforms to the schema Convert vis_type_timeseries client side code to Typescript #63593
  • Get rid of request facade in server code to provide a nicer extension point for search strategies (this should to be done before typing the server because otherwise a lot of legacy code has to be typed before it gets removed) [TSVB] Remove request facade and type server code #92964
  • Type server code to make sure it conforms to the schema
  • Change client code to only create well-formed config objects (e.g. no empty strings also allowed for number fields)
  • Write saved object migration to clean up ugly parts of schema
  • Lock down schema to necessary shape
  • Convert vis_type_timeseries client side code to Typescript
@flash1293 flash1293 added Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Feb 11, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@DianaDerevyankina DianaDerevyankina self-assigned this May 6, 2020
DianaDerevyankina added a commit to DianaDerevyankina/kibana that referenced this issue May 6, 2020
DianaDerevyankina added a commit that referenced this issue May 8, 2020
* Clean up TSVB turn Joi schema into kbn schema

Part of #57342

* Return validationTelemerty and logging part back

* Add schema.maybe() where it was missed
DianaDerevyankina added a commit to DianaDerevyankina/kibana that referenced this issue May 8, 2020
* Clean up TSVB turn Joi schema into kbn schema

Part of elastic#57342

* Return validationTelemerty and logging part back

* Add schema.maybe() where it was missed
DianaDerevyankina added a commit that referenced this issue May 8, 2020
* Clean up TSVB turn Joi schema into kbn schema

Part of #57342

* Return validationTelemerty and logging part back

* Add schema.maybe() where it was missed
DianaDerevyankina added a commit to DianaDerevyankina/kibana that referenced this issue Jun 8, 2020
DianaDerevyankina added a commit that referenced this issue Jun 25, 2020
* Clean up TSVB type client code to conform to the schema

Part of #57342

* Replace FieldDescriptor with IFieldType, add UIRestrictions interface

* Replace expect from chai with @kbn/expect, remove unnecessary type

* Add TimeseriesUIRestrictions type and refactor add_delete_buttons.test

* Replace some types with MetricsItemsSchema['values'] to avoid duplications

Co-authored-by: Elastic Machine <[email protected]>
DianaDerevyankina added a commit to DianaDerevyankina/kibana that referenced this issue Jun 25, 2020
* Clean up TSVB type client code to conform to the schema

Part of elastic#57342

* Replace FieldDescriptor with IFieldType, add UIRestrictions interface

* Replace expect from chai with @kbn/expect, remove unnecessary type

* Add TimeseriesUIRestrictions type and refactor add_delete_buttons.test

* Replace some types with MetricsItemsSchema['values'] to avoid duplications

Co-authored-by: Elastic Machine <[email protected]>
DianaDerevyankina added a commit that referenced this issue Jun 26, 2020
)

* Clean up TSVB type client code to conform to the schema

Part of #57342

* Replace FieldDescriptor with IFieldType, add UIRestrictions interface

* Replace expect from chai with @kbn/expect, remove unnecessary type

* Add TimeseriesUIRestrictions type and refactor add_delete_buttons.test

* Replace some types with MetricsItemsSchema['values'] to avoid duplications

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
@stratoula stratoula changed the title Clean up TSVB API interface Clean up TSVB API interface, convert to typescript Jun 2, 2023
@stratoula stratoula added the impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. label Jun 2, 2023
@stratoula stratoula added the technical debt Improvement of the software architecture and operational architecture label Jan 26, 2024
@stratoula
Copy link
Contributor

I am going to close this issue as we are in the phase of deprecating TSVB slowly so we don't intend to add more effort to tech dept tasks, unless this is super necessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:TSVB TSVB (Time Series Visual Builder) impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

No branches or pull requests

5 participants