Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nikosv opened this issue May 29, 2020 · 7 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@nikosv
Copy link

nikosv commented May 29, 2020

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 nikosv added the bug Something isn't working label May 29, 2020
@matchish matchish added the help wanted Extra attention is needed label May 29, 2020
@nikosv
Copy link
Author

nikosv commented Jun 1, 2020

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

@matchish
Copy link
Owner

matchish commented Jun 1, 2020

Cool, a pull requests for that would being highly appreciated. 🎉
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

$app['config']->set('scout.driver', ElasticSearchEngine::class);

@gousta
Copy link

gousta commented Jun 1, 2020

@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
Copy link
Owner

matchish commented Jun 2, 2020

I believe the solution is to not register commands if driver isn't Matchish\ScoutElasticSearch\Engines\ElasticSearchEngine

@matchish
Copy link
Owner

matchish commented Jun 2, 2020

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
Copy link

gousta commented Jun 3, 2020

@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
Copy link
Owner

matchish commented Jun 3, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants