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

InfluxDB: Add new truthiness operators (Is and Is Not) to InfluxQL Query Builder #77923

Merged
merged 10 commits into from
Nov 15, 2023

Conversation

btasker
Copy link
Contributor

@btasker btasker commented Nov 9, 2023

What is this feature?

Implements support for including query conditions based upon boolean field values using new Is and Is Not operators (see note below) (#77794).

Why do we need this feature?

There's a pre-existing issue where it's not possible because after submission, the value will be coerced to a string (e.g. false will be coerced to 'false').

Who is this feature for?

Users who want to use the InfluxQL Query Builder to build WHERE conditions based on Boolean fields

Which issue(s) does this PR fix?:

Fixes #77794

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@btasker btasker marked this pull request as ready for review November 9, 2023 14:12
@btasker btasker requested a review from a team as a code owner November 9, 2023 14:12
@btasker
Copy link
Contributor Author

btasker commented Nov 9, 2023

@itsmylife - this should be ready for review.

The latest 3 commits add type inference so that Is and Is Not work with string/int/float fields (if the key indicates a tag the value will always be quoted). If needed, they can safely be dropped/reverted out of the PR before merge so that it simply adds Is and Is Not.

The front-end code has also been updated so that correct translation occurs when the user switches between builder and raw mode (I didn't think to check that on the previous PR)

Copy link
Contributor

@itsmylife itsmylife left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good! ❤️
Could you please provide tests for influx_query_model too?
Thanks in advance!

@btasker
Copy link
Contributor Author

btasker commented Nov 13, 2023

👍 done!

@btasker
Copy link
Contributor Author

btasker commented Nov 14, 2023

@itsmylife FYI, that pending check just shows "Build is blocked, please, contact repo admin in order to proceed" (same on the other PR too).

@itsmylife
Copy link
Contributor

@btasker I will enable it when we'll fix the problem in our CI pipeline. There is an unrelated thing broken right now.

@btasker
Copy link
Contributor Author

btasker commented Nov 14, 2023

👍 Thanks - was just double checking I hadn't broken anything :)

@itsmylife
Copy link
Contributor

@btasker I merged the other PR and we got conflicts with this one. Could you please resolve them? I tried on github UI and failed :) And I don't want to mess your PR up.

Copy link
Contributor

@itsmylife itsmylife left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM it just needs the conflicts to resolve

@btasker
Copy link
Contributor Author

btasker commented Nov 15, 2023

👍 Sure will look at these a little later (I had a feeling we might get some)

@btasker
Copy link
Contributor Author

btasker commented Nov 15, 2023

Github's UI was failing me too, so I've rebased on main and resolved the conflicts

@itsmylife
Copy link
Contributor

@btasker now there is a linting issue. But we are quite close to merge it :)

+ yarn run prettier:check
[warn] public/app/plugins/datasource/influxdb/components/editor/query/influxql/visual/TagsSection.tsx
[warn] public/app/plugins/datasource/influxdb/influx_query_model.test.ts
[warn] public/app/plugins/datasource/influxdb/influx_query_model.ts
[warn] Code style issues found in 3 files. Run Prettier to fix.

@btasker
Copy link
Contributor Author

btasker commented Nov 15, 2023

Got it - looks like I accidentally introduced some additional whitespace whilst rebasing (lint-prettier doesn't get called during that process because there's no commit to trigger the hooks)

@itsmylife itsmylife merged commit f38f657 into grafana:main Nov 15, 2023
14 checks passed
@aangelisc aangelisc modified the milestones: 10.3.x, 10.2.3 Dec 21, 2023
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.

InfluxDB: It's not possible to include boolean fields in query conditions using the InfluxQL Editor
3 participants