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.

Sign up for GitHub

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

Allow to set custom headers #148

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow to set custom headers #148

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions rest-request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"net/http"
"net/url"
"path"
"strconv"
"strings"
)

Expand All @@ -37,10 +38,29 @@ var DefaultHTTPClient = http.DefaultClient

// Client uses Grafana REST API for interacting with Grafana server.
type Client struct {
baseURL string
key string
basicAuth bool
client *http.Client
baseURL string
key string
basicAuth bool
client *http.Client
customHeaders map[string]string
}

// SetCustomHeaders - set additional headers that will be sent with each request
func (r *Client) SetCustomHeaders(headers map[string]string) {
r.customHeaders = headers
}

// SetCustomHeader - set additional header that will be sent with each request
func (r *Client) SetCustomHeader(key, value string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like it will be unnecessary confusion for users to have two different APIs for the same purpose. Could we please remove this one as the other one suffices and is clearer i.e. the behaviour is very clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so, do you propose to leave SetCustomHeaders and remove SetCustomHeader? I think SetCustomHeader easier to use and will be mostly used, but SetCustomHeaders still useful if you want to rewrite and clear all custom headers.
WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that the typical way in Go to pass key/value pairs is to use map[KeyType]ValueType and it has quite a nice syntax. So, I personally don't see a reason to add some extra methods on top of that which simply do someMap[k] = v. If someone really wants this functionality then it probably won't be a huge inconvenience to store those headers somewhere on the user side 🤔 we could probably add a custom convenience function - GetCustomHeaders.

I really don't see a point in hiding from the users that it's a map[string]string underneath as that is what headers are, really 😄

if r.customHeaders == nil {
r.customHeaders = map[string]string{}
}
r.customHeaders[key] = value
}

// SetOrgIDHeader - set `X-Grafana-Org-Id` to specified value
func (r *Client) SetOrgIDHeader(oid uint) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if we should have this. It was undocumented for a long time and we have https://github.com/grafana-tools/sdk/blob/master/rest-org.go#L145-L146 for this purpose. IMO if someone wants this then they should really set it manually themselves.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/grafana-tools/sdk/blob/master/rest-org.go#L145-L146

this one actually differ, cause if you are an admin you need to switch context to use it, but it won't work in parallel at all

r.SetCustomHeader("X-Grafana-Org-Id", strconv.FormatUint(uint64(oid), 10))
}

// StatusMessage reflects status message as it returned by Grafana REST API.
Expand Down Expand Up @@ -105,6 +125,11 @@ func (r *Client) doRequest(ctx context.Context, method, query string, params url
if !r.basicAuth {
req.Header.Set("Authorization", r.key)
}
if r.customHeaders != nil {
for key, value := range r.customHeaders {
req.Header.Set(key, value)
}
}
req.Header.Set("Accept", "application/json")
req.Header.Set("Content-Type", "application/json")
req.Header.Set("User-Agent", "autograf")
Expand Down