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

- Prevent unnecessary api calls on Collection (index). #818 #820

Merged
merged 3 commits into from
Apr 9, 2024

Conversation

AbdullahFaqeir
Copy link
Contributor

Prevent unnecessary api calls on Collection (index) when updating/inserting ...etc. #818

@AbdullahFaqeir
Copy link
Contributor Author

I believe we also need to add a testing step for typesense, but I'm honestly not knowledgable in github actions.

@taylorotwell taylorotwell merged commit 8af517e into laravel:10.x Apr 9, 2024
13 checks passed
@AbdullahFaqeir AbdullahFaqeir mentioned this pull request Apr 29, 2024
saibotk added a commit to saibotk/scout that referenced this pull request Jul 12, 2024
In //laravel#820 we introduced an exists check method, which then lead to static state issues as described in laravel#845

Those where addressed in laravel#846

But now that the function always double-checks the existence, we can remove the exists check entirely and only rely on the Typesense server response for this state.

This "fixes" issues where the server already has a collection and the client would try to recreate it. E.g. when it has flushed the index and another process or worker then creates the collection.
saibotk added a commit to saibotk/scout that referenced this pull request Jul 12, 2024
In laravel#820 we introduced an exists check method, which then lead to static state issues as described in laravel#845

Those where addressed in laravel#846

But now that the function always double-checks the existence, we can remove the exists check entirely and only rely on the Typesense server response for this state.

This "fixes" issues where the server already has a collection and the client would try to recreate it. E.g. when it has flushed the index and another process or worker then creates the collection.
saibotk added a commit to saibotk/scout that referenced this pull request Jul 12, 2024
In laravel#820 we introduced an exists check method, which then lead to static state issues as described in laravel#845

Those where addressed in laravel#846

But now that the function always double-checks the existence, we can remove the exists check entirely and only rely on the Typesense server response for this state.

This "fixes" issues where the server already has a collection and the client would try to recreate it. E.g. when it has flushed the index and another process or worker then creates the collection.
saibotk added a commit to saibotk/scout that referenced this pull request Jul 12, 2024
In laravel#820 we introduced an exists check method, which then lead to static state issues as described in laravel#845

Those where addressed in laravel#846

But now that the function always double-checks the existence, we can remove the exists check entirely and only rely on the Typesense server response for this state.

This "fixes" issues where the server already has a collection and the client would try to recreate it. E.g. when it has flushed the index and another process or worker then creates the collection.
taylorotwell added a commit that referenced this pull request Aug 5, 2024
* refactor(typesense): remove unused exists checks

In #820 we introduced an exists check method, which then lead to static state issues as described in #845

Those where addressed in #846

But now that the function always double-checks the existence, we can remove the exists check entirely and only rely on the Typesense server response for this state.

This "fixes" issues where the server already has a collection and the client would try to recreate it. E.g. when it has flushed the index and another process or worker then creates the collection.

* Update TypesenseEngine.php

---------

Co-authored-by: Taylor Otwell <[email protected]>
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.

2 participants