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

[SIP-133] Update "Time Range" filter in dashboard #28489

Open
sundar68 opened this issue May 14, 2024 · 7 comments
Open

[SIP-133] Update "Time Range" filter in dashboard #28489

sundar68 opened this issue May 14, 2024 · 7 comments
Labels
change:backend Requires changing the backend change:frontend Requires changing the frontend sip Superset Improvement Proposal

Comments

@sundar68
Copy link

sundar68 commented May 14, 2024

[SIP-133] Proposal for Update "Time Range" filter in dashboard<title>

Motivation

Time Range filter contains "Last day" option it provides time range for yesterday.
I want the superset team to include "Today" in time range.

For "Today" time range should be From Yesterday date at current time -To today date at current time
It will be useful for getting last 24hrs data from current time.

Proposed Change

Include "Today" in frontend dropdown list for time range,
Modify "api/v1/time_range" api to get time range for "TODAY"

New dependencies

Migration Plan and Compatibility

Rejected Alternatives

@sundar68 sundar68 added the sip Superset Improvement Proposal label May 14, 2024
@dosubot dosubot bot added api Related to the REST API change:backend Requires changing the backend change:frontend Requires changing the frontend dashboard:filtersets Related to the filtersets of the Dashboard labels May 14, 2024
@rusackas
Copy link
Member

This doesn't seem like a SIP... and if it is, you'd need to fill out the rest of the template for it to be considered.

That said, I kind of agree here, and feel this could be interpreted as a bug. Superset offers both "last" and "previous" options, and we have a bit of an inconsistency here.

In "last" all the choices are essentially "last {granularity} until now" where "previous" is effectively "previous complete {granularity}. For example, "last month" is 2024-04-14 to 2024-05-14, whereas "previous month" is 2024-04-01 to 2024-05-01. To me, these seem sensible.

When it comes to the day option(s) however, we're a bit inconsistent. The "last day" option is really what should be called "previous calendar day" under the "previous" menu. For "last day" it seems like it should be "24-hours-before now"
to "now".

As a workaround, and potential inroads to a PR, you can currently do what you're seeking under the "Custom" tab, like so:
image

But I think I agree with you that this should be what "last day" actually does, and the current "last day" range should be moved over to a (new) "previous calendar day" option under "Previous".

Curious what others (@kasiazjc @villebro @michael-s-molina ) think about this. It might not be a hard feature to add, but could be a tricky migration for existing filters/charts/etc.

@rusackas rusackas removed api Related to the REST API dashboard:filtersets Related to the filtersets of the Dashboard labels May 14, 2024
@sundar68
Copy link
Author

@rusackas Thank you for your comment.

Yes, we can get from "Custom", but it's not a single click. We have to select "custom" and need tp select "Now" for END and modify days to 1 in START.

We can add new option like "Last 24hrs" where time range will be last 24 hours. It won't require more changes I guess.

For my usecase I added new option "Last 24hrs" and in time_range fetch api I just sending the time_range for last 24hrs in place of query params.


http://localhost:8088/api/v1/time_range/

query params:
q: '2024-05-14T13:14:18 : 2024-05-15T13:14:18'


response:
{
   "result": [
       {
           "since": "2024-05-14T13:14:18",
           "until": "2024-05-15T13:14:18",
           "timeRange": "2024-05-14T13:14:18 : 2024-05-15T13:14:18"
       }
   ]
}

@rusackas
Copy link
Member

I think adding that option would be a fantastic PR, and not be considered a "breaking change" or need a migration. Once we have "Last 24 Hours" in place, I think it would make sense to move the current "Last Day" implementation over to the "Previous" tab as "Previous calendar day" and assess what migrations (if any) need to be done for existing charts using that option.

@rusackas rusackas changed the title [SIP] Update "Time Range" filter in dashboard [SIP-133] Update "Time Range" filter in dashboard May 15, 2024
@rusackas
Copy link
Member

I'm not sure this requires a SIP... I think you can just go straight to a PR with the above idea, but I'll ping @michael-s-molina @kasiazjc @eschutho for a few more opinions on the matter. If you do want to carry this forward as a SIP, you should open a "Discuss" thread on the dev@ mailing list... say the word if you need help pursuing that.

@sfirke
Copy link
Contributor

sfirke commented May 17, 2024

I agree that this is inconsistent and that we should move the current "last day" to "previous day" when adding the new option. If a migration could do that automatically for existing charts, that would be fantastic.

Regarding naming, I think it's okay that "last 24 hours" and "previous day" don't have the same terminology because the definitions are different, if I understand. "Last 24 hours" is just like what it sounds like, "previous day" is the the most recent completed calendar day from midnight to midnight. Is that right?

I agree this should not be controversial or have enough different options to require the conversation of a SIP.

@sundar68
Copy link
Author

@rusackas
I think it would be better if you include "This Month", "This Year", "This Week", "Today".

Today- Should consider today morning 00:00 to present time.
This Week- This week starting to present day and time.
This Month- This month 1st to present day and time.
Similarly for "This Year"

@rusackas
Copy link
Member

I think the "Current" option being proposed in the PR linked right above this comment seems to scratch that itch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:backend Requires changing the backend change:frontend Requires changing the frontend sip Superset Improvement Proposal
Projects
Development

No branches or pull requests

3 participants