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

Provide alternatives to getConfig() #3114

Open
alexislefebvre opened this issue Apr 6, 2023 · 11 comments
Open

Provide alternatives to getConfig() #3114

alexislefebvre opened this issue Apr 6, 2023 · 11 comments

Comments

@alexislefebvre
Copy link

Description
In #2516, getConfig() was deprecated, but no replacement was suggested.

Example

We use https://packagist.org/packages/eightpoints/guzzle-bundle to use Guzzle in a Symfony project, with this config:

# config/packages/eight_points_guzzle.yaml
eight_points_guzzle:
    clients:
        example_api:
            base_url: "https://api.example.com/"
            options:
                timeout: 30
                http_errors: true
                headers:
                    User-Agent: "EightPointsGuzzleBundle/v7"
                    Content-Type: 'application/json'
                    TokenID: '%example_api_key%'
            plugin: null

The method that use Guzzle can return null early if no key is provided for this client, so that we can enable or disable the calls to the external API by changing the value of %example_api_key% in our different environments.

# src/ExampleClient.php

class ExampleClient
{
    public function __construct(
        private readonly \GuzzleHttp\Client $client,
    ) {
    }

    private function call(): ?array
    {
        if (!$this->client->getConfig()['headers']['TokenID']) {
            return null;
        }
        
        // …

        // the call to Guzzle is done here
        return json_decode(
            $this->client->post($uri, $parameters)->getBody(),
            true, 512, JSON_THROW_ON_ERROR
        );
    }
}

If getConfig() is removed in Guzzle 8, we won't be able to access to the config and disable these calls.

We could pass %example_api_key% to do the same thing but I think that it would be nice to rely only on the $client that already contain this value.

Additional context

Idea: add getHeaders() or something like getHeader('TokenID'), etc. to allow to access to the values that are accessible through getConfig() today with Guzzle 7.

See also #2514

@vaidas-lungis
Copy link

use Middleware and check if request headers has TokenId ?

@alexislefebvre
Copy link
Author

use Middleware and check if request headers has TokenId ?

Thanks, it looks interesting: https://docs.guzzlephp.org/en/stable/handlers-and-middleware.html#middleware

@GrahamCampbell
Copy link
Member

This is marked as deprecated, but probably I won't actually ever remove this. I just want to encourage people to not use it.

@srdjanpejic
Copy link

I'm also using this Symfony package, like mentioned in the issue's description, and now I'm not sure how to push middleware to the existing client, currently I'm doing it like this:

$oauth = new OAuth2Middleware();
$this->guzzleClient->getConfig('handler')->push($oauth);

With getConfig being deprecated is there an alternative to achieve the same functionality?

@nimafallahian
Copy link

I see you've mentioned you'll probably never remove this but the issue is I'm using phpstan to draw the attention to problematic lines of code in my codebase. I can ignore these lines but would prefer to have them out of deprecation notice or have an alternative to it. It would be wonderful if it's part of your plan to do one of these. :)

@GrahamCampbell
Copy link
Member

You can view this as technical debt in your code. We do not recommend using that function, but we appreciate many people do.

@alexislefebvre
Copy link
Author

I'm also using this Symfony package, like mentioned in the issue's description, and now I'm not sure how to push middleware to the existing client, currently I'm doing it like this:

$oauth = new OAuth2Middleware();
$this->guzzleClient->getConfig('handler')->push($oauth);

With getConfig being deprecated is there an alternative to achieve the same functionality?

Should it be asked and/or documented on the bundle repo? https://github.com/8p/EightPointsGuzzleBundle

@GrahamCampbell
Copy link
Member

You would need to create the handler then construct a guzzle client with said handler.

@GrahamCampbell
Copy link
Member

Much like in #3185.

@lkmorlan
Copy link

lkmorlan commented Jan 4, 2024

In Drupal, a Guzzle client can be injected. We need to know what its settings are so that we can create a client with our own settings based on that. I don't see a way around this. This is what we use getConfig() for.

@LeoWie93
Copy link

LeoWie93 commented Jan 9, 2024

@GrahamCampbell

This is marked as deprecated, but probably I won't actually ever remove this. I just want to encourage people to not use it.

What speaks against marking this function as internal insetad of depcrecated?

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

No branches or pull requests

7 participants