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

Range labels backport #4846

Merged
merged 12 commits into from
Sep 4, 2015
Merged

Range labels backport #4846

merged 12 commits into from
Sep 4, 2015

Conversation

w33ble
Copy link
Contributor

@w33ble w33ble commented Sep 2, 2015

Closes #4826, which was a backport issue for #4404

This is a backport of #4613 and #4677 to the 4.1 branch

  • Fixes labels on range aggregations
  • Add +/- Infinity labels
  • Range label tests

screenshot 2015-09-02 16 41 06

@w33ble
Copy link
Contributor Author

w33ble commented Sep 3, 2015

Should probably get a LGTM from @spalger on this one too, since it was all originally his code.

@lukasolson
Copy link
Member

screen shot 2015-09-03 at 11 13 30 am

Looks like the data is coming back and the data table is populating correctly, but the bars are not showing up correctly.

@lukasolson lukasolson assigned w33ble and unassigned lukasolson Sep 3, 2015
@w33ble
Copy link
Contributor Author

w33ble commented Sep 4, 2015

@lukasolson needed to backport #4677 too. Everything should be working now.

screenshot 2015-09-03 17 28 24

@lukasolson lukasolson assigned lukasolson and unassigned w33ble Sep 4, 2015
@lukasolson
Copy link
Member

Hmmm, I think there's still another commit backport missing, because now I'm seeing this bug when I create a range filter on a scripted field:

image

@lukasolson
Copy link
Member

Also, this fix will probably also need to be backported as part of this after the above is working: #4751

@lukasolson lukasolson assigned w33ble and unassigned lukasolson Sep 4, 2015
@w33ble w33ble force-pushed the range-labels-backport branch from a5ac2bd to 254b382 Compare September 4, 2015 19:40
key is an object with a toString prop, which needs to be removed - this worked in the 4.2 version by using a Symbol
@w33ble
Copy link
Contributor Author

w33ble commented Sep 4, 2015

@lukasolson good find! Scripted range filters were actually broken too - should all be fixed now.

screenshot 2015-09-04 13 24 57

@w33ble w33ble assigned lukasolson and unassigned w33ble Sep 4, 2015
@lukasolson
Copy link
Member

LGTM! Passing to @spalger for a second look.

@lukasolson lukasolson assigned spalger and unassigned lukasolson Sep 4, 2015
@spalger
Copy link
Contributor

spalger commented Sep 4, 2015

LGTM

spalger added a commit that referenced this pull request Sep 4, 2015
@spalger spalger merged commit 734d602 into elastic:4.1 Sep 4, 2015
@w33ble w33ble mentioned this pull request Sep 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants