Skip to content

dont use schemas in aggconfigs to output dsl - #26010

Merged
ppisljar merged 2 commits into
elastic:masterfrom
ppisljar:fix/visualize
Nov 22, 2018
Merged

ppisljar merged 2 commits into
elastic:masterfrom
ppisljar:fix/visualize

Conversation

@ppisljar

Copy link
Copy Markdown
Contributor

Summary

A couple of small fixes in visualize in preparation for #25996

  • set searchsourceid on vis
  • keep mapBounds in aggconfig json
  • dont use schemas in aggconfigs to output dsl

QA: no functional changes

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

- keep mapBounds in aggconfig json
- dont use schemas in aggconfigs to output dsl
- pass global filters from editor to visualize
@ppisljar ppisljar added review v7.0.0 Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v6.6.0 labels Nov 21, 2018
@elasticmachine

Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app

self.visState
);

self.vis.savedSearchId = self.savedSearchId;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we will need this later, but as we discussed with tim that we want to keep both rendering infrastructures for a while, i want to add small things like this in separately from the main PR that will introduce rendering thru canvas pipeline

describe('precision parameter', () => {

const PRECISION_PARAM_INDEX = 6;
const PRECISION_PARAM_INDEX = 7;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this changed due to added mapBounds parameter

write: _.noop
},
{
name: 'mapBounds',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we are adding this here, so mapBounds is part of json output for this aggregation. We are gonna need this in the pipeline.

super({
index: ['id'],
group: ['schema.group', 'type.name', 'schema.name'],
group: ['schema.group', 'type.name', 'type.type', 'schema.name'],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we should not use schema.group for outputing the dsl (in the pipeline we won't have schema definitions at that time). this should already be removed, seems this single thing slipped then.

@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@ppisljar

Copy link
Copy Markdown
Contributor Author

retest

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@lukeelmers lukeelmers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code LGTM

@ppisljar
ppisljar merged commit 8f88091 into elastic:master Nov 22, 2018
ppisljar added a commit to ppisljar/kibana that referenced this pull request Nov 22, 2018
@ppisljar
ppisljar deleted the fix/visualize branch November 22, 2018 03:29
@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v6.6.0 v7.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants