Use xml to configure discovered clients #183
Conversation
| $httpClient = new Reference($httpClient); | ||
| if (!empty($httpClient) && $httpClient !== 'auto') { | ||
| $container->setAlias('httplug.auto_discovery.auto_discovered_client', $httpClient); | ||
| } elseif (null === $httpClient) { |
There was a problem hiding this comment.
Because possible values are null, auto or any configured client name. So the first case handle the "any configured client name" and the elseif the null case which means remove the definition from the container.
| $asyncHttpClient = new Reference($asyncHttpClient); | ||
| if (!empty($asyncHttpClient) && $asyncHttpClient !== 'auto') { | ||
| $container->setAlias('httplug.auto_discovery.auto_discovered_async', $asyncHttpClient); | ||
| } elseif (null === $asyncHttpClient) { |
| $container->setAlias('httplug.auto_discovery.auto_discovered_async', $asyncHttpClient); | ||
| } elseif (null === $asyncHttpClient) { | ||
| $container->removeDefinition('httplug.auto_discovery.auto_discovered_async'); | ||
| $container->removeDefinition('httplug.collector.auto_discovered_async'); | ||
| } | ||
|
|
||
| if (null === $httpClient && null === $asyncHttpClient) { |
There was a problem hiding this comment.
Yes, If we have no discovered client (sync and async), we remove the useless listener.
There was a problem hiding this comment.
looks a lot more readable to me.
an alternative to removing the definitions from the container could be to use separate files that we only load when the respective client is active. given the size of the xml file, i think it could be a good idea to split those out. wdyt?
| } | ||
|
|
||
| $httpClient = new Reference($httpClient); | ||
| if (!empty($httpClient) && $httpClient !== 'auto') { |
There was a problem hiding this comment.
if ($httpClient !== 'auto') {
//...
if (!empty($httpClient) {
//...
}
}There was a problem hiding this comment.
I don't used to write nested conditions (object calisthenics ❤️) but I will make an exception.
|
I've update the code to make #184 tests fine. About conditional xml loading, I don't think it worth it. I would need to load 5 different files ( I think it would be easier later as the profiling stuff will be handled differently (in the PluginClientFactory injected factory callback). |
|
Thank you for reviews! |
This move discovered clients configuration from
HttplugExtensiontoservices.xmlanddata-collector.xml.To Do