fix: handle URL length limits in Typesense engine for large courses#263
Conversation
When indexing a large course with many content blocks, the Typesense
search engine could hit two related URL-length failures:
1. search(): Building an exclusion filter like
'id:!=[id1, id2, ...]' for hundreds of item IDs produced a URL
query string that exceeded httpx's internal limit, raising:
httpx.InvalidURL: URL component 'query' too long
An existing fallback for RequestMalformed ('Query string exceeds
max allowed length') did not cover this client-side failure.
Fix: also catch httpx.InvalidURL (narrowed to 'too long' message)
and route both cases to the multi-search POST endpoint, which has
no URL length constraint.
2. remove(): Deleting many stale documents built a single
'id: [id1,id2,...]' filter sent as a DELETE query parameter, which
is equally subject to URL length limits.
Fix: chunk deletions into batches of _MAX_IDS_PER_DELETE_BATCH
(100) IDs per request.
Also fix a latent bug in get_search_params(): the exclude_dictionary
loop iterated unconditionally, raising AttributeError when the caller
passed exclude_dictionary=None (the documented default).
Tests added for all three fixes in search/tests/test_typesense.py.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Thanks for the pull request, @blarghmatey! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Three improvements based on a review of the Typesense filtering and data-types documentation: 1. Use the documented '![]' (is-not-any-of) filter operator instead of '!=[...]'. The operators table in https://typesense.org/docs/guide/tips-for-filtering.html#available-operators documents '![]' as the canonical multi-value exclusion operator; '!=[...]' combines the single-value inequality operator with an array argument and is not an officially documented form. 2. Add range_index: True to all datetime fields (start, end, enrollment_start, enrollment_end, start_date). The schema-parameters reference documents this flag as enabling an optimised index for range-based numerical filtering, which is exactly how date fields are queried (e.g. 'show only active courses'). 3. Strip HTML from content sub-fields before indexing. The Typesense data-types guide explicitly recommends storing plain text rather than raw HTML markup so that tags are not indexed as searchable tokens, wasting in-memory index space and polluting search results. A new _strip_html / _strip_html_from_content helper pair is applied inside process_document() only to the nested 'content' object, since other fields (course keys, org names, etc.) are clean identifiers that should not be modified. Tests added for all three changes in search/tests/test_typesense.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
this is great, but really should be done for all search engines, and/or we should change |
bradenmacdonald
left a comment
There was a problem hiding this comment.
Really nice work! Thanks so much.
I tested this and it seems to be working well, although I needed open-craft/tutor-contrib-typesense#8 to get it working with Tutor and #267 to get the "Discover Courses" search working. Neither of those issues are related to this PR however.
Summary
When indexing a large course with many content blocks (e.g. 500+), the Typesense search engine can fail with URL-length errors. This PR fixes three scale/robustness issues and separately applies three improvements identified from a review of the Typesense filtering and data-types documentation.
Scale / URL-length fixes
Problem 1 –
search()raiseshttpx.InvalidURL(reported bug)An existing fallback for
RequestMalformed("Query string exceeds max allowed length") covered the server-side case but not this client-side one. Both are now caught and routed to the multi-search POST endpoint, which has no URL-length constraint. Thehttpx.InvalidURLcatch is narrowed to messages containing "too long" so that unrelated URL errors are still raised normally.Problem 2 –
remove()has the same URL-length riskremove()sent a singleDELETErequest withfilter_by: id: [id1,id2,...]for every stale document. The Typesense Python client passesfilter_byas a URL query parameter, so a large stale-item list triggers the same failure. Documents are now deleted in batches of at most_MAX_IDS_PER_DELETE_BATCH(100) IDs per request.Problem 3 –
get_search_params()crashes whenexclude_dictionary=NoneThe function signature documents
exclude_dictionary=Noneas the default, but the loop body iterated unconditionally, raisingAttributeError: 'NoneType' object has no attribute 'items'. ANoneguard has been added.Typesense best-practices improvements (from docs review)
2 –
range_index: Trueon datetime fieldsThe schema parameters reference documents
range_indexas enabling an optimised index for range-based numerical filtering — exactly how date fields are queried (e.g. "active courses only", "enrollment window"). Added tostart,end,enrollment_start,enrollment_end, andstart_date.3 – Strip HTML from
content.*fields before indexingThe Typesense data-types guide explicitly recommends stripping HTML markup before indexing so that tags are not treated as searchable tokens and do not bloat the in-memory index. XBlock
index_dictionary()methods can return raw HTML in content fields. A new_strip_html/_strip_html_from_contenthelper is applied inprocess_document()to the nestedcontentobject only; other fields (course keys, org names, etc.) are left unmodified.Testing
search/tests/test_typesense.pyis a new test module covering all six changes:TypesenseEngine.search()normal path,httpx.InvalidURLfallback,RequestMalformedfallback, and that unrelated errors are still re-raisedTypesenseEngine.remove()empty list, small batch, exact batch size, large batch (multiple requests), and correct per-batch ID slicing_strip_htmland_strip_html_from_contenthelpers (tag removal, entity unescaping, whitespace normalisation, nested structures, non-string values)TypesenseEngine.process_document()HTML stripping in content field, non-content fields untouched, missing content field handledAll new tests pass; pre-existing failures in the Elasticsearch and Meilisearch suites are unrelated to these changes.
Related
The edx-platform
remove_deleted_itemscaller has also been updated in openedx/edx-platform to avoid building a large exclusion filter entirely — it now paginates through all indexed items for the course, diffs in Python, and callssearcher.remove()only on the stale IDs.