Remove the superseded v5/v6/v7 search schema revisions#3523
Remove the superseded v5/v6/v7 search schema revisions#3523jonathangreen wants to merge 6 commits into
Conversation
v8 subclasses SearchSchemaRevision directly and defines its full mapping inline, instead of chaining off v7 the way earlier revisions did. With the chain broken, old revisions can be deleted once production no longer uses them. v8 also pins number_of_shards to 1. Earlier indexes inherited a 5-primary-shard count from the Elasticsearch 6.x era, carried forward through every reindex because nothing set it explicitly. Per-library indexes are well under a gigabyte, so a single primary shard is correct, and the setting is immutable after index creation. number_of_replicas is left to runtime management.
v8 now sets index.search.slowlog.threshold.* on every index it creates, so slow query- and fetch-phases are written to the cluster slow log. Paired with the SEARCH_SLOW_LOGS publishing wired up in the hosting-playbook, these entries reach the domain's CloudWatch log group for investigation. The thresholds are dynamic settings, so they establish a baseline at index creation that can still be retuned on a live index without a reindex.
v8 now sets number_of_replicas explicitly (to 1) alongside number_of_shards, so a newly created index has a fully deterministic configuration rather than relying on an inherited cluster default. One replica matches what every production index already runs and is also the OpenSearch default, so this is operationally a no-op. The setting is dynamic and can still be retuned at runtime for a larger topology.
v8 is self-contained and is the only revision production creates indexes from, so the older chained revisions (v7 -> v6 -> v5) are no longer referenced and can be deleted. This removes their modules, drops them from the revision directory, and repoints the remaining test/mock usages at v8. The v8 module test absorbs the author char-filter test that previously lived against v5 in test_external_search, and drops its v7-equivalence test now that v7 is gone.
|
Claude finished @jonathangreen's task in 2m 32s —— View job Code Review
SummaryThis is a clean, well-scoped deletion that does exactly what it says: removes the superseded v5/v6/v7 revision modules and their tests, trims |
With v5/v6/v7 removed in this PR, the docstring no longer needs to recount the old inheritance chain to justify v8 being self-contained.
Greptile SummaryThis PR removes the now-superseded v5, v6, and v7 search schema revision modules and their test files, following the self-contained v8 revision introduced in #3522. The
Confidence Score: 5/5Safe to merge once the release containing v8 has shipped and production has reindexed — correctly kept as a draft until then. All v5/v6/v7 references are completely removed from both source and tests with no dangling imports or dead code. The character-filter test is faithfully ported to TestSearchV8, the mock's backing document is updated to v8, and the revision directory contains exactly one entry. No logic was changed, only deleted. The deployment guard (keep draft until production migrates) is explicit in the PR description. No files require special attention. Important Files Changed
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feature/search-v8 #3523 +/- ##
=====================================================
- Coverage 93.46% 93.44% -0.02%
=====================================================
Files 512 509 -3
Lines 46561 46464 -97
Branches 6344 6343 -1
=====================================================
- Hits 43516 43418 -98
- Misses 1968 1969 +1
Partials 1077 1077 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I'm going to leave this in draft until its safe to merge, but its ready for a code review now, then it can just wait to get merged once approved. |
2 participants
Description
Stacked on top of #3522 (
feature/search-v8). Removes the superseded v5/v6/v7 search schema revisions now that v8 is self-contained and is the only revision production creates indexes from.src/palace/manager/search/v5.py,v6.py, andv7.py, along with their dedicated tests (test_search_v6.py,test_search_v7.py).SearchRevisionDirectory, leavingREVISIONS = [SearchV8()].MockSearchSchemaRevisionLatestmapping document and aname_for_indexdocstring example.test_external_search.py) intoTestSearchV8, and drops v8's v7-equivalence test now that v7 no longer exists to compare against.Important
Do not merge until the release containing #3522 has shipped and production has reindexed onto v8. This is the "release 2" half of the two-stacked-PRs workflow in CLAUDE.md. While any production index is still on v5/v6/v7, the running application must be able to load those revisions (e.g. to read the current index version), so their removal is only safe once nothing in production references them. Kept in draft until then.
Motivation and Context
#3522 made v8 self-contained specifically so that older revisions could be deleted once they fell out of use, instead of accumulating indefinitely behind the inheritance chain. This PR is that cleanup. Opening it now (in draft) keeps the follow-up visible so the removal isn't forgotten after the v8 release goes out.
How Has This Been Tested?
Ran the affected suites under the docker tox environment —
tests/manager/search/,tests/manager/scripts/test_initialization.py, andtests/manager/celery/tasks/test_search.py(the latter two exerciseMockSearchSchemaRevisionLatest, whose backing document now comes from v8). All 171 tests pass.mypyis clean on the changed source.Checklist