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

feat: support configured headers as request options #4

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

Conversation

p4veI
Copy link

@p4veI p4veI commented Jun 20, 2024

Hello, I came around an issue with the Client Config, for our use case we need to set a Host header for ceph rgw different from the base_uri provided for the client constructor, while you can set headers in the config which is then parsed and should be included in the request an alternative Host header has a buggy behaviour and is replaced by the host used as base_uri.

So I wonder if we could allow to set the headers in each request separately, using headers like this should avoid this bug.

The behaviour is also described in this issue in guzzle (guzzle/guzzle#1297 - marked as solved but reported as still occurring). I realise this might be an edge case but makes sense to me to use the headers config directly like this..

@lbausch
Copy link
Owner

lbausch commented Jun 22, 2024

Thank you for your PR!
Can you share some example code showing how you would use this feature?
We would also need some tests.

@p4veI
Copy link
Author

p4veI commented Jun 23, 2024

Thanks for your prompt response! I'm now using my fork with this change the following way:

       $client = Client::make(
            'endpoint', //this is e.g. admin.rgw-domain.com
            'key',
            'secret',
            Config::make([
                'httpClientConfig' => [
                    'connect_timeout' => 300,
                ],
                'httpClientHeaders' => [
                    'Host' => 'real-endpoint', //this is e.g. rgw-domain.com
                ],
            ]),
        );

And then it allows me to use all resources identically as before we changed our configuration that requires this:

        $response = $client->user()->info($userName);
        // do something 
        return $response

I can confirm sending the host header like this works the expected way. I'll be happy to make any changes to resolve this.
I wanted to avoid defining the host header in each resource call separately (e.g. by passing propagation via options like $client->user()->info($userName, $optionsWithHeader)).

@p4veI
Copy link
Author

p4veI commented Jun 24, 2024

@lbausch I included a simple test for setting host header via config, and tested this test also fails on master branch, let me know if there's something else you'd like to change/include etc.

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.

2 participants