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

Adds influxdb v2 query runner. #6646

Merged
merged 5 commits into from
Dec 12, 2023

Conversation

fabrei
Copy link
Contributor

@fabrei fabrei commented Dec 4, 2023

  • Adds test cases
  • Adds influxdb v2 icon
  • Updates python dependencies

What type of PR is this?

  • Refactor
  • Feature
  • Bug Fix
  • New Query Runner (Data Source)
  • New Alert Destination
  • Other

Description

It uses influxdb-client python library which is compatible with influxdb v2.

It supports ssl connection to influxdb as well.

How is this tested?

  • Unit tests (pytest, jest)
  • E2E Tests (Cypress)
  • Manually
  • N/A

Related Tickets & Documents

#5391

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

- Adds test cases
- Adds influxdb v2 icon
- Updates python dependencies
@masayuki038
Copy link
Collaborator

Hi @fabrei, Thanks for the PR.
The lint check failed. Could you check them?

@fabrei
Copy link
Contributor Author

fabrei commented Dec 5, 2023

Hi @fabrei, Thanks for the PR. The lint check failed. Could you check them?

Oh yes. Sorry, I didn't notice the linting and formatting check in test env. I fixed it :)

Copy link

codecov bot commented Dec 5, 2023

Codecov Report

Merging #6646 (f363a9c) into master (9bbdb4b) will increase coverage by 0.19%.
The diff coverage is 96.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6646      +/-   ##
==========================================
+ Coverage   62.57%   62.77%   +0.19%     
==========================================
  Files         161      162       +1     
  Lines       13184    13261      +77     
  Branches     1797     1805       +8     
==========================================
+ Hits         8250     8324      +74     
- Misses       4649     4651       +2     
- Partials      285      286       +1     
Files Coverage Δ
redash/query_runner/influx_db_v2.py 96.10% <96.10%> (ø)

Copy link
Collaborator

@masayuki038 masayuki038 left a comment

Choose a reason for hiding this comment

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

@fabrei Regarding updates, it looks good from my end.

Did you run a manual test? I don't have InfluxDB2 env. So, I will rely on the results of your manual tests.

And please add "InfluxDB2" (I don't know a formal name) in https://github.com/getredash/redash#supported-data-sources.

@fabrei
Copy link
Contributor Author

fabrei commented Dec 5, 2023

@fabrei Regarding updates, it looks good from my end.

Did you run a manual test? I don't have InfluxDB2 env. So, I will rely on the results of your manual tests.

Yes, I tested the query runner manually with an influx database version 2. I tested it, with and without using ssl. It worked fine for me.

And please add "InfluxDB2" (I don't know a formal name) in https://github.com/getredash/redash#supported-data-sources.

I added it, but used "InfluxDBv2", so that it's clear, that it's a query runner for influx database version 2.

Copy link
Collaborator

@masayuki038 masayuki038 left a comment

Choose a reason for hiding this comment

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

@fabrei Thanks for your reply. It is okay from my end.

@guidopetri Please review this PR if you have some time.

@TobiasGrether
Copy link

Is there any ETA when this will be in the docker builds? I wanted to switch from grafana to redash but we need Influx v2 support

@masayuki038
Copy link
Collaborator

masayuki038 commented Dec 8, 2023

@TobiasGrether This is in 1-week-review. I think this will be merged in a few days if there are no issues.

@masayuki038 masayuki038 requested a review from konnectr December 9, 2023 15:48
@masayuki038
Copy link
Collaborator

There were no issues in 1-week-review. I am going to merge this.

@masayuki038 masayuki038 enabled auto-merge (squash) December 12, 2023 13:02
@masayuki038 masayuki038 disabled auto-merge December 12, 2023 13:03
@masayuki038 masayuki038 merged commit 66ef942 into getredash:master Dec 12, 2023
15 checks passed
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.

4 participants