Skip to content

Feature/use api key #1

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

josselinchevalay
Copy link

Hello,

i think it can be intresting use api key looks like : http://docs.grafana.org/http_api/auth/#create-api-token instead of user credentials.

Regards

Copy link
Contributor

@f0y f0y left a comment

Choose a reason for hiding this comment

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

Thank you for contribution!

@komarovd95
Copy link
Contributor

@josselinchevalay Please, add description of apiKey property to README.md
Anyway, thank you for your contribution

/**
* Grafana api key
*/
String getApiKey() { return apiKey; }
Copy link
Contributor

Choose a reason for hiding this comment

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

@josselinchevalay Please, make each statement on a line by itself. Like other getters.

String getApiKey() {
  return apiKey;
}

@@ -78,8 +78,12 @@ private static String getHttpResponseBodyAsString(HttpResponse response) throws
}

private String getAuthHeader() {
return "Basic " + Base64.getEncoder().encodeToString(
(grafanaUploadSettings.getUser() + ':' + grafanaUploadSettings.getPassword()).getBytes(UTF_8));
if(grafanaUploadSettings.getApiKey().isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@josselinchevalay i think it will be better to invert this if statement. And you should check that apiKey property is not null before. Smth like this:

if (grafanaUploadSettings.getApiKey() != null && !grafanaUploadSettings.getApiKey().isEmpty()) {
  return "Basic " + grafanaUploadSettings.getApiKey();
}
// old way

Copy link
Author

Choose a reason for hiding this comment

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

@komarovd95 i'm not sure if we no declare Api key we can use Basic auth else we use Bearer and api key

Copy link
Author

Choose a reason for hiding this comment

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

@codecov
Copy link

codecov bot commented Mar 4, 2019

Codecov Report

Merging #1 into master will decrease coverage by 2.03%.
The diff coverage is 50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master       #1      +/-   ##
============================================
- Coverage     82.85%   80.82%   -2.04%     
- Complexity       33       34       +1     
============================================
  Files             8        8              
  Lines           140      146       +6     
  Branches          5        5              
============================================
+ Hits            116      118       +2     
- Misses           19       22       +3     
- Partials          5        6       +1
Impacted Files Coverage Δ Complexity Δ
...lugins/grafana/dashboard/impl/DashboardSender.java 69.56% <50%> (-6.63%) 3 <1> (ø)
.../grafana/dashboard/impl/GrafanaUploadSettings.java 86.2% <50%> (-5.8%) 7 <1> (+1)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44bbbf6...1637ea0. Read the comment docs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants