Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Explore: Add transformations to correlation data links #61799
Explore: Add transformations to correlation data links #61799
Changes from all commits
4b7f38d
54fa1e1
35f2c6a
80369bd
2ec4f1f
d76656c
2aef4d9
daa9370
4250d75
9e979f7
c838601
b1cf69b
3d1e571
527901c
db3a85e
003c5d4
409901c
ed3f375
0cccdd1
776a2b5
e68003c
cb79371
d9dd95b
b299c1d
2cab225
220a9ee
091149e
23c77fe
df36e99
321d489
e695298
8abfa8a
c743f42
ec1e426
023fd66
2a08a5a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
this slightly changes the behaviour of a bunch of things, like the panel model inspector in dashboards and variables, did we test it's safe? (especially for variables as
false
and0
would have resulted in an empty string, but"false"
and"0"
now, not sure if that's even possible tho)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.
I just couldn't imagine why we would want true to evaluate to "true" and false to evaluate to an empty string. Let's look at the usages of this function outside how it is used in this PR
features/dashboard/state/PanelModel.ts
- passes the entire model into this function, and that model is a required parameter. I am extremely confident that PanelModel will never evaluate to false or 0, there are a lot of required fields in it.features/variables/utils.ts
- safe-stringifies the first arg, concatenates all args together split by a space except the last one and checks that string has matches for any of the three formats for variables. I do not believe that having false or 0 evaluate to an empty string or not will have any impact on this, because the safe-stringify only runs on the first argument, and in all cases a variable reference must start with either$
or[
which is not possible with a false or 0 scenario.features/variables/inspect/utils.ts
- Similar to the above, we safe-stringify a value and check if it matches any of the variable patterns. False or 0 or empty string will all not match. That value is not used again - it will use the matching groups instead.plugins/datasource/prometheus/datasource.tsx
- This will only run if the if statement depending on the same value is true, so if the value passed in is 0 or false, it will never run in the first placeI'm very confident based on those usages that this logic will not impact anything.