Skip to content

Conversation

flash1293
Copy link
Contributor

Fixes #63081

For some parameters the TSVB UI always sets strings, but numbers should also be allowed and work just fine.

This PR relaxes the types a little in this regard.

It also resets the failed validation counter for 7.7.1

@flash1293 flash1293 added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v7.7.1 labels Apr 15, 2020
@flash1293 flash1293 marked this pull request as ready for review April 15, 2020 14:59
@flash1293 flash1293 requested review from a team and simianhacker April 15, 2020 14:59
Copy link
Member

@simianhacker simianhacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@flash1293 flash1293 added Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// labels Apr 16, 2020
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@wylieconlon wylieconlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code change LGTM, although I did not test clicking the link generated by the Metrics Explorer, I looked at how metrics explorer generates links. They are generating URLs that are parsed by us, which is why the string validation seems to be failing. For example, vis:(aggs:!(),params:(axis_formatter:number,axis_min:0, was being parsed as a number instead of string.

axis_max: stringOptionalNullable,
axis_min: stringOptionalNullable,
axis_max: stringOrNumberOptionalNullable,
axis_min: stringOrNumberOptionalNullable,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing that we are still using a mix of both types within our own code, for example: seriesGroup.axis_min = seriesGroup.axis_min || 0;

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@flash1293 flash1293 removed the v7.7.1 label Apr 27, 2020
@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@flash1293 flash1293 merged commit 45aa090 into elastic:master Apr 27, 2020
@flash1293 flash1293 deleted the tsvb-validation-fix-3 branch April 27, 2020 15:58
flash1293 added a commit to flash1293/kibana that referenced this pull request Apr 27, 2020
jloleysens added a commit to jloleysens/kibana that referenced this pull request Apr 28, 2020
…bana into pipeline-editor-part-mvp-2

* 'feature/ingest-node-pipelines' of github.com:elastic/kibana: (152 commits)
  [Ingest pipelines] Simulate pipeline (elastic#64223)
  Ability to get scoped call cluster from alerting and action executors (elastic#64432)
  Add editApp and editPath to embeddable (elastic#64297)
  TSVB validation: Allow numeric values for axes (elastic#63553)
  [ML] Fixing optional plugin dependency types (elastic#64450)
  [Logs UI] Alerting (elastic#62806)
  [Endpoint] Show Policy Status on Host Details using Policy Response API (elastic#64116)
  [Maps] update LayerWizard previewLayer to take layerDescriptor instead of ISource (elastic#64461)
  Remove SO root property index signature (elastic#64434)
  [ML] Functional tests - stabilize job row details validations (elastic#64503)
  [Ingest] Add Global settings flyout (elastic#64276)
  Bump cypress dev-dependency from 4.2.0 to 4.4.1 (elastic#64408)
  Migrate saved object of type url to kibana platform (elastic#64043)
  [NP] Migrate ui capabilities (elastic#64185)
  Bump karma-mocha dev-dependency from 1.3.0 to 2.0.0 (elastic#64407)
  Migrate kql_telemetry saved object registration to Kibana platform (elastic#64149)
  Remove SO autocreateindex error and error page (elastic#64037)
  Fix issue with yarn.lock (elastic#64496)
  Bump @hapi/boom dependency from 7.4.2 to 7.4.11 (elastic#64433)
  Bump gonzales-pe dev-dependency from 4.2.4 to 4.3.0 (elastic#64401)
  ...

# Conflicts:
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form.tsx
#	x-pack/plugins/ingest_pipelines/public/shared_imports.ts
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) release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.8.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Metrics] TSVB warning contains outdated configuration

5 participants