-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: add Elasticsearch Engine #3
base: main
Are you sure you want to change the base?
feat: add Elasticsearch Engine #3
Conversation
Currently, I can't get it working.
After changing the constructor of public function __construct(Model $realModel)
{
parent::__construct([]);
$this->realModel = $realModel;
} to: public function __construct(Model $realModel = null)
{
parent::__construct([]);
if($realModel) {
$this->realModel = $realModel;
}
} I get the following error:
|
You cannot change the constructor of If I understand correctly, from my quick look at the error trace and the source code of the package on GitHub https://github.com/matchish/laravel-scout-elasticsearch/blob/master/src/ElasticSearch/Params/Bulk.php#L44 it's related to the In my extension, I actually re-implemented every part of the code from Scout that called that method, hence why I created a no-op version that throws an exception every time in flarum-ext-scout/src/ScoutModelWrapper.php Lines 201 to 204 in 4e1eecd
The reason why you don't hit the intended exception on the first attempt seems to be because instead of calling The most reliable solution would probably to re-implement that Otherwise, to make the code works without any override, you could probably add a I hope this gives you some hints. I don't have a lot of time at the moment, but if this works well this looks like an interesting addition and I'll give it a try! |
Actually, now that I think of it, maybe not all of the code related to soft-deletes was re-implemented. I would have to check again, but maybe in the Scout original code it might do
instead of that elastic package
The order is important, because the |
Thanks for the hints!
Yes, I just checked the flipped if ((testing directly in vendor)) and it is working. |
I used this method ^ I also created a PR for |
This reverts commit da02d70.
Sorry, I didn't get any opportunity to check back on this. Would the PR be ready as-is? |
Yes |
As in the title