Add history plugin to the auto discovery client #36
Conversation
|
i guess this is not wrong per se, as the .default is just an alias. but i wonder if we should not handle multiple clients and show multiple icons with tooltip per client service name? or log all requests to one common collection and allow to filter by client service name? |
No, the
I do not think we should have multiple toolbar icons. But I do think we should requests and responses per client on the profiler page and in the toolbar pop up. I can write another PR about that. |
Not sure if multiple icons is a good idea. My toolbar is big enough already 😝 I agree with @Nyholm, it makes sense on the profiler page. (Don't know how doctrine does this with multiple entity managers, maybe we could look at that) |
| @@ -88,6 +89,11 @@ protected function configureClients(ContainerBuilder $container, array $config) | |||
| // Alias the first client to httplug.client.default | |||
| if ($first !== null) { | |||
| $container->setAlias('httplug.client.default', 'httplug.client.'.$first); | |||
| } elseif (isset($config['_inject_collector_plugin'])) { | |||
| // No client was configured. Make sure to inject history plugin to the auto discovery client. | |||
| $container->register('httplug.client', PluginClient::class) | |||
There was a problem hiding this comment.
was httplug.client not available at all before? or did this happen somewhere else? i am a bit confused by this.
There was a problem hiding this comment.
httplug.client was an alias before this line. This register it as a service.
|
right, makes more sense to have one report and allow to filter by client. i guess that overlaps somewhat with #26 as well. we don't need to fix this in this PR, lets just make the incremental improvement of not missing out on the auto discovery client. |
|
Yeah, I agree that there is some overlapping. |
Add history plugin to the auto discovery client
|
thanks. i added a note to #26 in the description. |
If you do not configure httplug at all it will use the auto discovery client. That client should also have the history plugin injected if you use the toolbar.
Im not 100% confident in this PR. We will as always have two identifiers for the default client:
httplug.clientandhttplug.client.default. This PR makeshttplug.clienta plugin client but not the other.Is this okey? The implications would be that you do not see the toolbar when using no configuration and the
httplug.client.defaultservice id.