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

Timepicker conf #3908

Closed
wants to merge 6 commits into from
Closed

Timepicker conf #3908

wants to merge 6 commits into from

Conversation

Filirom1
Copy link
Contributor

It closes #2874 and #2758 and also makes refreshIntervalDefaults and refreshIntervals configurable.

Because of circular dependency with config -> courier/data_source/doc_source -> courier/data_source/_abstract -> courier/fetch/fetch -> courier/fetch/strategy/search -> timefilter -> config, I was forced to to some refactoring.

Now strategies are not instanciated by generic components like fetch, courier/data_source/_abstract. But are instantiated by specialized components : courier, doc_source, search_source, looper/doc

I hope this refactoring will suit you.

@rashidkpc
Copy link
Contributor

While I like the intention here, we don't want this implemented as JSON jammed into an advanced settings parameter. We'd like to see this implemented as a configuration dialog somewhere near the time picker.

Also the refactoring should be separated from the work of introducing configuration to the timepicker

@spalger
Copy link
Contributor

spalger commented May 21, 2015

👍 to the courier changes. Would like that in a separate pr as well.

@Filirom1
Copy link
Contributor Author

Ok, I will create a dedicated PR for the refactoring worldline@e7704a6

@Filirom1
Copy link
Contributor Author

@spalger PR #3915 created

I understand that the quickRanges and refreshIntervals definitions are quite huge JSON.

On the other hand, timeDefaults (63aa432) and refreshIntervalDefaults (43eae32) are quite small.

Are you OK for 2 other PR for this 2 small JSON configuration ? One PR will close #2874

@rashidkpc
Copy link
Contributor

I think those 2 would be reasonable to have in advanced config, though we need to look at the implementation of the refreshInterval object, display and section really shouldn't be needed

@Filirom1
Copy link
Contributor Author

@rashidkpc PR #3976 and #3977 created.

I don't know how to handle the configuration of quick_ranges. I think, you want these values to be saved in the configuration, but not to be displayed in the advanced settings parameters.

It could be interesting to generate the Quick time period selection from the Relative and Absolute panels:

  • In the Relative and Absolute panels, we could add a button to export this time period in the Quick panel.
  • In the Quick panel we will need to have a delete button and to be able to change the order of the time-periods.

I am not sure I will have the opportunity to do this implementation because it's a little bit more complex than expected.

If I am able to change the quick_ranges with an ES update query, I will be happy with that.
I only want to display the following time-periods:

from: 'now-1d/d', to: 'now-1d/d', display: 'Yesterday'
from: 'now-2d/d', to: 'now-2d/d', display: 'Day before yesterday'
from: 'now-7d/d', to: 'now-7d/d', display: 'This day last week'
from: 'now-1M/d', to: 'now-1M/d', display: 'This day last month'
from: 'now-1y/d', to: 'now-1y/d', display: 'This day last year'

You could notice that the time periods I am looking at could not be generated from the Relative and Absolute panels.

@rashidkpc
Copy link
Contributor

Thanks for #3976, #3977 and #3915. I'm going to go ahead and close this ticket but we can brainstorm more in #2758

@rashidkpc rashidkpc closed this Jun 3, 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.

Timepicker: change default
3 participants