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

fix: add globalheader to client#getNodesInfo() #24

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

Conversation

AkisAya
Copy link

@AkisAya AkisAya commented Aug 13, 2024

i use go-elasticsearch to visit elasticsearch servers, and due to the restrictions of my company, i have to add a custom header to the request. go-elasticsearch config and underlying transport client config provides a global header field to do this thing, like the bode below

c, err := es8.NewTypedClient(es8.Config{
	Addresses:            url,
	Username:             name,
	Password:             pwd,
	DiscoverNodesOnStart: true,
	Header:               globalHeader,
})

it works when i just perform a search, because the underlying transport client set the global header before performing a request like this

func (c *Client) Perform(req *http.Request) (*http.Response, error) {
	var (
		res *http.Response
		err error
	)
        ...
	// Update request
	c.setReqUserAgent(req)
	c.setReqGlobalHeader(req)
        ...
}

c.setReqGlobalHeader(req) is used to set the global header.

but when it comes to discoverNodes(), the global header is not set, leading to discoverNodes failure in my case

func (c *Client) getNodesInfo() ([]nodeInfo, error) {
	var (
		out    []nodeInfo
		scheme = c.urls[0].Scheme
	)

	req, err := http.NewRequest("GET", "/_nodes/http", nil)
	...

	c.setReqURL(conn.URL, req)
	c.setReqAuth(conn.URL, req)
	c.setReqUserAgent(req)

	res, err := c.transport.RoundTrip(req)
        ...
}

I think this is a mistake, and this pr add c.setReqGlobalHeader(req) to getNodesInfo() to set global header

@AkisAya
Copy link
Author

AkisAya commented Aug 13, 2024

Hi @Anaethelion , would you mind reviewing this PR whenever you are available?

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

Successfully merging this pull request may close these issues.

1 participant