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

Fix searchable queue configuration #840

Closed
wants to merge 4 commits into from
Closed

Fix searchable queue configuration #840

wants to merge 4 commits into from

Conversation

tklie
Copy link

@tklie tklie commented Jun 20, 2024

This PR fixes how the Searchable trait determines whether to queue jobs. Also, the published configuration file is updated to be compatible with the current version of the trait.


The Searchable trait currently determines the queue connection and queue using the following two methods:

    public function syncWithSearchUsing()
    {
        return config('scout.queue.connection') ?: config('queue.default');
    }

    public function syncWithSearchUsingQueue()
    {
        return config('scout.queue.queue');
    }

However, the config file that is published by this package only contained the following config option:

return [
    'queue' => env('SCOUT_QUEUE', false),
];

This has been updated to the keys actually used by the trait:

return [
    'queue' => [
        'connection' => env('SCOUT_CONNECTION'),
        'queue' => env('SCOUT_QUEUE', false),
    ],
]

Subsequently, another update to the Searchable trait itself was necessary, since this used the old scout.queue key to determine if jobs should be queued. It now correctly uses scout.queue.queue:

    public function queueMakeSearchable($models)
    {
        if ($models->isEmpty()) {
            return;
        }

        if (! config('scout.queue.queue')) {
            $models->first()->makeSearchableUsing($models)->first()->searchableUsing()->update($models);
            return;
        }

        dispatch((new Scout::$makeSearchableJob($models))
                ->onQueue($models->first()->syncWithSearchUsingQueue())
                ->onConnection($models->first()->syncWithSearchUsing()));
    }

Edit: The same was done for the queueRemoveFromSearch method.

@taylorotwell
Copy link
Member

Would break all existing apps.

@tklie
Copy link
Author

tklie commented Jun 21, 2024

Will be fixed in future versions though? Because as it stands right now, the Searchable trait is inconsistent within itself, using two different config keys.

To clarify: You can use it as is right now by setting the entire scout.queue option in your config file to false. But having the config file like this

return [
    'queue' => [
        'connection' => env('SCOUT_CONNECTION'),
        'queue' => env('SCOUT_QUEUE', false),
    ],
]

and disabling queueing via an env variable is not possible right now, because scout.queue will always be truthy since it's an array.

@bedus-creation
Copy link

@taylorotwell it would be nice, if we could fix this by adding another options possibly like connection ?

'connection' => env('SCOUT_CONNECTION')

and

 public function syncWithSearchUsing()
 {
    return config('scout.connection') ?: config('queue.default');
}

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.

3 participants