search

matchish / laravel-scout-elasticsearch

Search among multiple models with ElasticSearch and Laravel Scout
MIT License
705 stars 113 forks source link

[BUG] Scout import overrides scout driver in config when package is installed #119

Open nikosv opened 4 years ago

nikosv commented 4 years ago

Describe the bug We are using multiple scout drivers in our app and more specifically elasticsearch in staging server and algolia in production server. It seems that while algolia is set in production as default scout engine the php artisan scout:import uses the elasticsearch import of your package instead of that of algolia. Funny thing is that php artisan scout:flush on the other hand works great and flushes data in algolia as expected.

To Reproduce Steps to reproduce the behavior:

  1. require matchish/laravel-scout-elasticsearch and add provider to config/app.php
  2. Edit config/scout.php and set algolia as default search engine
  3. Run a command php artisan scout:import 'App\Models\User'
  4. See error related to matchish/laravel-scout-elasticsearch package instead of algolia

Expected behavior Correct Expected Behaviour: Either to import to algolia (if credentials are set correctly) or show an algolia related error message What actually happens: An elasticsearch error will occur

Version "laravel/framework": "^6.18", "laravel/scout": "^8.0", "matchish/laravel-scout-elasticsearch": "4.0.x",

nikosv commented 4 years ago

@matchish We could probably help with that. Any guidance on where should we focus on or any suggestions?

matchish commented 4 years ago

Cool, a pull requests for that would being highly appreciated. :tada: https://github.com/matchish/laravel-scout-elasticsearch/blob/master/CONTRIBUTING.

Here I set a driver for tests. You have to override it in your tests https://github.com/matchish/laravel-scout-elasticsearch/blob/dfcc90d3356275e512ef509dcdc3d0e92df71e22/tests/TestCase.php#L35

gousta commented 4 years ago

@matchish Since what we want to accomplish here is that when config('scout.driver') is NOT equal to Matchish\ScoutElasticSearch\Engines\ElasticSearchEngine then we practically want to prevent the package from loading entirely.

I guess that could happen in one of the following 2 providers, but I'm not sure:

src/ElasticSearchServiceProvider.php
src/ScoutElasticSearchServiceProvider.php

What would you suggest?

matchish commented 4 years ago

I believe the solution is to not register commands if driver isn't Matchish\ScoutElasticSearch\Engines\ElasticSearchEngine https://github.com/matchish/laravel-scout-elasticsearch/blob/dfcc90d3356275e512ef509dcdc3d0e92df71e22/src/ScoutElasticSearchServiceProvider.php#L52

matchish commented 4 years ago

Does it solve the issue if you add this package to dev section of composer file?Then you can install without dev dependencies on production

gousta commented 4 years ago

@matchish Well since we try our staging environment to be 100% identical to the production environment (for greater debugging) it won't solve the issue.

I believe a check for the config('scout.driver') value before registering the commands would work just fine for our needs

matchish commented 4 years ago

Also you can try to make providers deffered https://laravel.com/docs/7.x/providers#deferred-providers But I'm not sure about drawbacks.