Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Adds ssl support for prometheus query runner. #6657

Merged
merged 6 commits into from
Dec 17, 2023

Conversation

fabrei
Copy link
Contributor

@fabrei fabrei commented Dec 11, 2023

  • Adds possibilty to upload and use of ssl cert, key and ca file in redash ui

What type of PR is this?

  • Feature

Description

All versions are compatible.

How is this tested?

  • Manually
  1. Created a new prometheus data source.
  2. Filled out necessary fields.
  3. Uploaded ssl cert, key and ca file.
  4. Saved data source.
  5. Clicked on "Test Connection".

Related Tickets & Documents

Asked in discussions to add this feature without issue.

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

image

- Adds possibilty to upload and use of ssl cert, key and ca file in redash ui
Copy link

codecov bot commented Dec 11, 2023

Codecov Report

Merging #6657 (6181d8f) into master (9bbdb4b) will increase coverage by 0.71%.
Report is 1 commits behind head on master.
The diff coverage is 96.50%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6657      +/-   ##
==========================================
+ Coverage   62.57%   63.28%   +0.71%     
==========================================
  Files         161      162       +1     
  Lines       13184    13314     +130     
  Branches     1797     1817      +20     
==========================================
+ Hits         8250     8426     +176     
+ Misses       4649     4599      -50     
- Partials      285      289       +4     
Files Coverage Δ
redash/query_runner/prometheus.py 95.86% <96.96%> (+54.55%) ⬆️
redash/query_runner/influx_db_v2.py 96.10% <96.10%> (ø)

... and 1 file with indirect coverage changes

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.

Hi @fabrei, thanks for your PR.

Your update seems to have some points not covered by the unit test. Can you add tests?

@fabrei
Copy link
Contributor Author

fabrei commented Dec 12, 2023

Hi @fabrei, thanks for your PR.

Your update seems to have some points not covered by the unit test. Can you add tests?

Hi @masayuki038,

I added some tests. Hope it works now with test coverage :)

@fabrei
Copy link
Contributor Author

fabrei commented Dec 12, 2023

Ah, damn the timezone O_o

The prometheus query runner does not use any timezone while calculating start and end time. It uses the local timezone on the system. In the CI it is UTC, which differs from my local one.

- Dynamically calculates timestamps in testcases to be robust in
  different timezones.
- Adds now datetime function to make it more testable.
@fabrei
Copy link
Contributor Author

fabrei commented Dec 12, 2023

Ah, damn the timezone O_o

The prometheus query runner does not use any timezone while calculating start and end time. It uses the local timezone on the system. In the CI it is UTC, which differs from my local one.

Now the testcases should be fixed.

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 updates! It is very helpful.
Looks good from my end. Please wait for a few days.

@guidopetri @konnectr Could you check this if you have some time?

@masayuki038
Copy link
Collaborator

I am going to merge this since there are no issues.

@masayuki038 masayuki038 enabled auto-merge (squash) December 17, 2023 11:52
@masayuki038 masayuki038 disabled auto-merge December 17, 2023 11:52
@masayuki038 masayuki038 merged commit 58bf96c into getredash:master Dec 17, 2023
13 checks passed
harveyrendell pushed a commit to pushpay/redash that referenced this pull request Jan 8, 2025
* Adds ssl support for prometheus query runner.

- Adds possibilty to upload and use of ssl cert, key and ca file in redash ui

* Extends test cases for prometheus query runner.

- Adds secret attribute to configuration schema.

* Fixes wrong timestamps in different timezones in prometheus' testcases.

- Dynamically calculates timestamps in testcases to be robust in
  different timezones.
- Adds now datetime function to make it more testable.

* Fixes timestamp in prometheus' testcases which can be wrong depending on timezone.

---------

Co-authored-by: Masayuki Takahashi <masayuki038@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants