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

Improve performance for sample listing index #2273

Merged
merged 18 commits into from
Mar 13, 2023

Conversation

ramonski
Copy link
Contributor

@ramonski ramonski commented Mar 12, 2023

Description of the issue/feature this PR addresses

This PR improves the performance of the listing_searchable_text ZCTextIndex of the sample catalog by adding only explicit terms into the index, instead of all the metadata columns defined.

Currently, the following values are part of the index:

  • Sample ID
  • Batch ID
  • Client ID
  • Client Name
  • Client Order Number
  • Client Reference
  • Client Sample ID
  • Sample Type Title
  • Sample Point Title

This improves the performance by 10% when creating new samples.

Furthermore, the stop words are kept in the Lexicon.

Current behavior before PR

All metadata columns are used for the sample listing_searchable_text index

Desired behavior after PR is merged

Only the above mentioned values are used for the listing_searchable_text index

--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.

@ramonski ramonski added Improvement 🔧 Cleanup 🧹 Code cleanup and refactoring labels Mar 12, 2023
@ramonski ramonski requested a review from xispa March 12, 2023 15:41
@xispa xispa merged commit 75e40c3 into 2.x Mar 13, 2023
@xispa xispa deleted the sample-catalog-zctext-index-performance branch March 13, 2023 11:30
@toropok
Copy link
Contributor

toropok commented Mar 29, 2023

@ramonski @xispa
Hi,

That seems you're intentionally removed get_searchable_text_tokens call from Samples indexer. How do you suppose to extend Sample entity custom search fields? Earlier custom ListingSearchableTextProvider class has to be implemented.

thanks!

@ramonski
Copy link
Contributor Author

Hi @toropok,
apologies, we overlooked this functionality.
Would you mind to hook it into the sample indexer directly?
The get_searchable_text_tokens will be sooner or later removed then.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup 🧹 Code cleanup and refactoring Improvement 🔧
Development

Successfully merging this pull request may close these issues.

3 participants