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

Don't reload query when closing a cell #18455

Closed
russorat opened this issue Jun 11, 2020 · 4 comments
Closed

Don't reload query when closing a cell #18455

russorat opened this issue Jun 11, 2020 · 4 comments

Comments

@russorat
Copy link
Contributor

similar to #18401

we already have the results, so there's no need to re-run the query when closing a dashboard cell editor modal.

@asalem1
Copy link
Contributor

asalem1 commented Jun 17, 2020

@russorat more specifically, did you want us to not reload the one cell that was closed, or did you want to not reload all the queries when closing a cell?

@russorat
Copy link
Contributor Author

@asalem1 We should definitely not rerun every query on the dashboard when they close a cell in most cases.

At a high level, we should only re-run queries automatically when absolutely needed. If there is larger re-work in patterns to accomplish this instead of a bandaid, let's consider it.

  • User made changes to their query in the cell editor but did not re-run the query before closing
  • We need to re-run the query when they save & close the cell to reflect their changes
  • If they clicked the red x to cancel their changes, we should display the previous result prior to opening the cell, so there shouldn't be a re-query in this case.
  • User made change to the variables or time range in the cell editor
  • In this case, we probably automatically re-ran their query for them in the cell editor (we do so when they change time range or variable values), but when they close the cell, those changes to variable values and time range are changed for the entire dashboard. That would mean we need to refetch all the visible cells on the dashboard EXCEPT the one they were editing (since we already ran that one).

Are there other scenarios we should consider here?

@asalem1
Copy link
Contributor

asalem1 commented Jun 18, 2020

I think you just about covered it. I'll post more here if I can think of any other use cases

@asalem1
Copy link
Contributor

asalem1 commented Jul 21, 2020

This should be addressed in #19072 (given that the configuration event is within the valid time range of cache validation)

@desa desa closed this as completed Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants