csarrazi
commented
4 years ago Thanks @emarref. Ideally, I would use a different option to entirely prevent the default_client logic, as this means that false becomes a "magic value", which I want to prevent if possible.
Something like disable_default_client, which would be a booleanNode with default value false.
Could you also add a test, and also document the feature?
This will be released as port of a minor version, so in the longer term, the default_client and disable_default_client will be hosted in a dedicated configuration node, which could be enabled/disabled.
The
default_clientoption is used to autowire instances of a guzzle client. However, in a project that has multiple guzzle clients, this can lead to bugs as mentioned by @BPScott in https://github.com/csarrazi/CsaGuzzleBundle/pull/222#discussion_r158629335.We recently ran into this very problem. We have a product that interacts with several third party APIs, and each of these APIs has a guzzle client. A new class was added that has a dependency on an explicit guzzle client, but the specific client to pass was accidentally not configured. This lead to an incorrect client being injected into the new class.
Conceptually, none of the guzzle clients we have should be considered the "default" one, as they're all "siblings" and do very different things. I understand that this configuration option was implemented similar to how Doctrine provides a default entity manager, but I'd argue Doctrine has a very different use-case to HTTP clients.
This pull request introduces the ability to disable the default client configuration. This pull request lets you set
default_client: falsein your configuration. As a result, if no guzzle client is explicitly configured for an argument, an exception is thrown by the container. However, leaving this configuration option blank will behave as the library currently does - injecting the first guzzle client that is found.