Skip to content

Conversation

@vntw
Copy link

@vntw vntw commented Nov 18, 2016

This PR adds a DataCollector that records all transactions within a Symfony application.

In debug mode, the \AlgoliaSearch\Client will be replaced by Algolia\AlgoliaSearchBundle\Debug\DebugClient. With this, it's possible to:

  • Fetch transactions in your functional tests:
// Test extending WebTestCase
$client = static::createClient();
$client->enableProfiler();
// Do something, e.g. controller request
$algoliaDataCollector = $client->getProfile()->getCollector('algolia.client_data_collector');
$transactions = $algoliaDataCollector->getTransactions();
// Assert transactions
  • Push mocked responses into a response stack that will be returned by the DebugClient if requests are disabled (disable_requests config):
// algolia.client -> DebugClient
$container->get('algolia.client')->pushResponse(['my' => 'response']);

Toolbar
screen shot 2016-11-18 at 11 15 37

Profiler
screen shot 2016-11-18 at 11 25 18

@maxiloc
Copy link
Contributor

maxiloc commented Dec 24, 2016

@venyii Thanks for this. We'll review that as soon as possible

@cjean-fr
Copy link
Contributor

Interesting, any news?

@rayrutjes
Copy link
Member

This looks great. I do think though that this kind of package should be released as a separate package. @venyii are you interested in shipping it?

If not possible with the current design of the bundle, we will need to make sure in a future release this kind of integration is possible.

I'm closing this for the time being. Let us know if you need any help releasing this @venyii .

Thanks for your support and your patience.

@rayrutjes rayrutjes closed this Mar 21, 2017
@vntw
Copy link
Author

vntw commented Mar 22, 2017

Hi @rayrutjes, to be honest, I don't think this should be a separate package. A DataCollector is usually included as part of a Symfony bundle for profiling/debugging purposes in the Symfony Web Debug Toolbar, not a single package.

See the official FrameworkBundle, SecurityBundle or 3rd-party bundles like SncRedisBundle as examples.

Could you elaborate why you'd want to separate these?

Thanks!

@rayrutjes
Copy link
Member

You want check this out @julienbourdeau ? I think we should move forward.

@piotrplenik
Copy link
Contributor

Hi guys, agree with @venyii.
Created separated bundle for that:
https://packagist.org/packages/jupeter/datacollector-algolia-search-bundle
I will be appreciated for any comments.

@julienbourdeau
Copy link
Contributor

Hi @jupeter,

I assume you meant that you agree with @rayrutjes. What did you choose to make it a separate package?

@piotrplenik
Copy link
Contributor

Hi @julienbourdeau,
yep - agree with @rayrutjes. Sorry for mistake.
About choose, pragmatic thing - i was requirement from my current project to implement DataCollector for Algolia and it was much easier to do it as separate bundle.
Matter of your future decisions:

  • if you prefer to have in this bundle DataCollector, I will remove the package in future;
  • if you want have new package in "algolia" github - please take it;

@julienbourdeau
Copy link
Contributor

Personally, I like having only one package as long as it doesn't add overhead in production.

The debug extension will not work without AlgoliaBundle and its code also relies on AlgoliaBundle. If for some reasons we change the signature of doRequest, we will need to update 2 separates codebase:

  • the Git history is a bit harder to track
  • it can results in dependency issues (if you update only one of the libs)

Also, I think it's a really cool feature so I'd rather see activated by default 😄

@alcaeus
Copy link
Contributor

alcaeus commented Sep 28, 2017

@julienbourdeau any update on this one?

@julienbourdeau julienbourdeau removed the v2 label Mar 5, 2018
@julienbourdeau julienbourdeau changed the base branch from master to 2.x August 28, 2018 10:07
@julienbourdeau julienbourdeau merged commit 217ce21 into algolia:2.x Aug 28, 2018
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.

7 participants