-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
fix(reduce-variable-hydration): filtering unused variables before hydrating them #18974
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blocking this over the app state being... too much nonsense. gonna need to offline this to make sure our testing strategy is inline.
…rs, need to add a test
Co-authored-by: Michael Desa <mjdesa@gmail.com>
d5409d7
to
ccedb87
Compare
ui/src/variables/actions/thunks.ts
Outdated
} | ||
return [] | ||
} | ||
|
||
// TODO: make this context aware |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you todid this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: this PR is about hydrateVariables
which is different than hydrateVars
. all of this extra mocking around the time machine is because of the selector we use to grab the current view when in data explorer mode (which does a bunch of stuff just to return the default view), which is currently the convention in our codebase
Problem
Over-hydrating variables whenever any queries are run
Solution
Filter out the unused variables before passing in the array of variables to update in order to reduce the graph dependencies that are ultimately returned and reduce the amount of queries that are performed in turn.