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 ValueParser.DateMath aware of timezone setting #12886

Merged
merged 2 commits into from Aug 19, 2015

Conversation

Projects
None yet
4 participants
@cbuescher
Member

cbuescher commented Aug 14, 2015

This PR adds a timezone field to ValueParser.DateMath that is
set to UTC by default but can be set using the existing constructors.
This makes it possible for extended bounds setting in DateHistogram
to also use date math expressions that e.g. round by day and apply
this rounding in the time zone specified in the date histogram
aggregation request.

Closes #12278

Aggregations: Make ValueParser.DateMath aware of timezone setting
This PR adds a timezone field to ValueParser.DateMath that is
set to UTC by default but can be set using the existing constructors.
This makes it possible for extended bounds setting in DateHistogram
to also use date math expressions that e.g. round by day and apply
this rounding in the time zone specified in the date histogram
aggregation request.

Closes #12278

@cbuescher cbuescher changed the title from Aggregations: Make ValueParser.DateMath aware of timezone setting to Make ValueParser.DateMath aware of timezone setting Aug 14, 2015

@jpountz

This comment has been minimized.

Show comment
Hide comment
@jpountz

jpountz Aug 18, 2015

Contributor

LGTM

Contributor

jpountz commented Aug 18, 2015

LGTM

@feltnerm

This comment has been minimized.

Show comment
Hide comment
@feltnerm

feltnerm commented Aug 19, 2015

💯 ! 🍻 @cbuescher

@feltnerm

This comment has been minimized.

Show comment
Hide comment
@feltnerm

feltnerm Aug 19, 2015

Any chance this will make it into the latest 1.x? Would be great!

feltnerm commented Aug 19, 2015

Any chance this will make it into the latest 1.x? Would be great!

@cbuescher

This comment has been minimized.

Show comment
Hide comment
@cbuescher

cbuescher Aug 19, 2015

Member

@feltnerm Sadly, I wouldn't count on it. Currently working towards 2.0, only major bugfixes get backported to the 1.x branches.

Member

cbuescher commented Aug 19, 2015

@feltnerm Sadly, I wouldn't count on it. Currently working towards 2.0, only major bugfixes get backported to the 1.x branches.

@cbuescher cbuescher added the v2.1.0 label Aug 19, 2015

cbuescher added a commit that referenced this pull request Aug 19, 2015

Merge pull request #12886 from cbuescher/fix/12278
Make ValueParser.DateMath aware of timezone setting

@cbuescher cbuescher merged commit 0fc96ed into elastic:master Aug 19, 2015

1 check passed

CLA Commit author is a member of Elasticsearch
Details
@cbuescher

This comment has been minimized.

Show comment
Hide comment
@cbuescher

cbuescher Aug 19, 2015

Member

@jpountz @clintongormley any opinion if this should be backported to 2.0?

Member

cbuescher commented Aug 19, 2015

@jpountz @clintongormley any opinion if this should be backported to 2.0?

@jpountz

This comment has been minimized.

Show comment
Hide comment
@jpountz

jpountz Aug 19, 2015

Contributor

It's a bug fix and risks look low to me so I would say yes.

Contributor

jpountz commented Aug 19, 2015

It's a bug fix and risks look low to me so I would say yes.

@cbuescher

This comment has been minimized.

Show comment
Hide comment
@cbuescher

cbuescher Aug 21, 2015

Member

Also backported to 2.0 branch with 8ba2c9b

Member

cbuescher commented Aug 21, 2015

Also backported to 2.0 branch with 8ba2c9b

@cbuescher cbuescher added the v2.0.0 label Aug 21, 2015

@clintongormley clintongormley added v2.0.0-beta1 and removed v2.0.0 labels Aug 25, 2015

@clintongormley clintongormley removed the v2.1.0 label Nov 22, 2015

@cbuescher cbuescher deleted the cbuescher:fix/12278 branch Mar 31, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment