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.

Sign up for GitHub

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

Jump to bottom

Refactor search index to improve connection handling #192

Merged
merged 3 commits into from
Jul 30, 2024
Merged

Refactor search index to improve connection handling #192

merged 3 commits into from
Jul 30, 2024

Conversation

tylerhutcherson
Copy link
Collaborator

We were leaning on a hack to run some async code in a sync setting. This was both dangerous and an anti-pattern. We also needed to refactor some of the shared and non-shared content between the BaseSearchIndex and derivatives.

@tylerhutcherson tylerhutcherson added the bug Something isn't working label Jul 30, 2024
rbs333
rbs333 approved these changes Jul 30, 2024
View reviewed changes
Copy link
Collaborator

@rbs333 rbs333 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like improvement to me. Key takeaway to use the new decorated .set_client

@tylerhutcherson
Copy link
Collaborator Author

Looks like improvement to me. Key takeaway to use the new decorated .set_client

Yes, for AsyncSearchIndex. Because we have to run a bit of async code to properly, and safely set it up, that's the correct way rather than the initial hack we had in place.

@tylerhutcherson tylerhutcherson merged commit 67eee3d into main Jul 30, 2024
20 checks passed
@tylerhutcherson tylerhutcherson deleted the fix/RAAE-203/refactor-search-index branch July 30, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rbs333 rbs333 rbs333 approved these changes

@bsbodden bsbodden Awaiting requested review from bsbodden

@justin-cechmanek justin-cechmanek Awaiting requested review from justin-cechmanek

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tylerhutcherson @rbs333