Skip to content

Commit

Permalink
add 'trove_search_in_flats' feature flag
Browse files Browse the repository at this point in the history
and tidy up the getting of strategy for search
  • Loading branch information
aaxelb committed Aug 23, 2023
1 parent bd53f7b commit 8d16765
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 42 deletions.
5 changes: 1 addition & 4 deletions api/search/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,7 @@ def _handle_request(self, request):
if 'scroll' in queryparams:
return http.HttpResponseForbidden(reason='Scroll is not supported.')
try:
specific_index = IndexStrategy.get_for_searching(
requested_index_strategy,
with_default_fallback=True,
)
specific_index = IndexStrategy.get_for_sharev2_search(requested_index_strategy)
except exceptions.IndexStrategyError as error:
raise http.Http404(str(error))
try:
Expand Down
5 changes: 1 addition & 4 deletions api/views/feeds.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,7 @@ def title(self, obj):
def get_object(self, request):
self._order = request.GET.get('order')
elastic_query = request.GET.get('elasticQuery')
self._index_strategy = IndexStrategy.get_for_searching(
request.GET.get('indexStrategy'),
with_default_fallback=True,
)
self._index_strategy = IndexStrategy.get_for_sharev2_search(request.GET.get('indexStrategy'))

if self._order not in {'date_modified', 'date_updated', 'date_created', 'date_published'}:
self._order = 'date_modified'
Expand Down
2 changes: 1 addition & 1 deletion project/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ def split(string, delim):
'CLUSTER_SETTINGS': ELASTICSEARCH8_CLUSTER_SETTINGS,
},
})
DEFAULT_INDEX_STRATEGY_FOR_SEARCHING = (
DEFAULT_INDEX_STRATEGY_FOR_LEGACY_SEARCH = (
'sharev2_elastic5'
if ELASTICSEARCH5_URL
else (
Expand Down
1 change: 1 addition & 0 deletions share/models/feature_flag.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class FeatureFlag(models.Model):
# flag name constants:
ELASTIC_EIGHT_DEFAULT = 'elastic_eight_default'
IGNORE_SHAREV2_INGEST = 'ignore_sharev2_ingest'
TROVE_SEARCH_IN_FLATS = 'trove_search_in_flats'

# name _should_ be one of the constants above, but that is not enforced by `choices`
name = models.TextField(unique=True)
Expand Down
48 changes: 31 additions & 17 deletions share/search/index_strategy/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,26 +79,40 @@ def get_specific_index(cls, specific_indexname: str) -> 'IndexStrategy.SpecificI
raise IndexStrategyError(f'unrecognized indexname "{specific_indexname}"')

@classmethod
def get_for_searching(cls, requested_name=None, *, with_default_fallback: bool = False) -> 'IndexStrategy.SpecificIndex':
if requested_name is not None:
try: # could be a strategy name
return cls.get_by_name(requested_name).pls_get_default_for_searching()
def get_for_sharev2_search(cls, requested_name=None) -> 'IndexStrategy.SpecificIndex':
if requested_name:
_name = requested_name
else:
_name = (
'sharev2_elastic8'
if FeatureFlag.objects.flag_is_up(FeatureFlag.ELASTIC_EIGHT_DEFAULT)
else settings.DEFAULT_INDEX_STRATEGY_FOR_LEGACY_SEARCH
)
try: # could be a strategy name
return cls.get_by_name(_name).pls_get_default_for_searching()
except IndexStrategyError:
try: # could be a specific indexname
return cls.get_specific_index(_name)
except IndexStrategyError:
try: # could be a specific indexname
return cls.get_specific_index(requested_name)
except IndexStrategyError:
raise IndexStrategyError(f'unknown name: "{requested_name}"')
if with_default_fallback:
return cls.get_for_searching(cls._default_strategyname_for_searching())
raise ValueError('either provide non-None requested_name or with_default_fallback=True')
raise IndexStrategyError(f'unknown name: "{_name}"')

@classmethod
def _default_strategyname_for_searching(cls) -> str:
return (
'sharev2_elastic8'
if FeatureFlag.objects.flag_is_up(FeatureFlag.ELASTIC_EIGHT_DEFAULT)
else settings.DEFAULT_INDEX_STRATEGY_FOR_SEARCHING
)
def get_for_trove_search(cls, requested_name=None) -> 'IndexStrategy.SpecificIndex':
if requested_name:
_name = requested_name
else:
_name = (
'trove_indexcard_flats'
if FeatureFlag.objects.flag_is_up(FeatureFlag.TROVE_SEARCH_IN_FLATS)
else 'trove_indexcard'
)
try: # could be a strategy name
return cls.get_by_name(_name).pls_get_default_for_searching()
except IndexStrategyError:
try: # could be a specific indexname
return cls.get_specific_index(_name)
except IndexStrategyError:
raise IndexStrategyError(f'unknown name: "{_name}"')

@classmethod
def _load_from_settings(cls, index_strategy_name, index_strategy_settings):
Expand Down
2 changes: 1 addition & 1 deletion tests/api/test_elasticsearch.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def test_search(self):
with mock.patch('api.search.views.IndexStrategy') as mock_IndexStrategy:
mock_handle_search = (
mock_IndexStrategy
.get_for_searching
.get_for_sharev2_search
.return_value
.pls_handle_search__sharev2_backcompat
)
Expand Down
2 changes: 1 addition & 1 deletion tests/api/test_feeds.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def fake_items(self, Graph):
json.loads(formatted_item)
for formatted_item in formatted_items
]
with mock.patch('api.views.feeds.IndexStrategy.get_for_searching') as mock_get_for_searching:
with mock.patch('api.views.feeds.IndexStrategy.get_for_sharev2_search') as mock_get_for_searching:
mock_strategy = mock_get_for_searching.return_value
mock_strategy.pls_handle_search__sharev2_backcompat.return_value = {
'hits': {
Expand Down
18 changes: 8 additions & 10 deletions tests/share/search/index_strategy/test_base_index_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ def test_get_by_specific_indexname(self, mock_elastic_clients, expected_strategy
with pytest.raises(IndexStrategyError):
IndexStrategy.get_specific_index(bad_indexname)

@pytest.mark.django_db
def test_get_by_request(self, mock_elastic_clients, fake_elastic_strategies):
IndexStrategy.clear_strategy_cache()
for strategy_name in mock_elastic_clients.keys():
index_strategy = IndexStrategy.get_by_name(strategy_name)
good_requests = [
Expand All @@ -55,17 +57,13 @@ def test_get_by_request(self, mock_elastic_clients, fake_elastic_strategies):
''.join((index_strategy.indexname_prefix, 'foo')),
]
for good_request in good_requests:
specific_index = IndexStrategy.get_for_searching(good_request)
specific_index = IndexStrategy.get_for_sharev2_search(good_request)
assert isinstance(specific_index, index_strategy.SpecificIndex)
assert specific_index.index_strategy is index_strategy
if good_request == strategy_name:
assert specific_index == index_strategy.pls_get_default_for_searching()
else:
assert specific_index.indexname == good_request
# bad calls:
with pytest.raises(IndexStrategyError):
IndexStrategy.get_for_searching('bad-request')
with pytest.raises(ValueError):
IndexStrategy.get_for_searching()
with pytest.raises(ValueError):
IndexStrategy.get_for_searching(requested_name=None)
IndexStrategy.get_for_sharev2_search('bad-request')
with pytest.raises(IndexStrategyError):
IndexStrategy.get_for_sharev2_search()
with pytest.raises(IndexStrategyError):
IndexStrategy.get_for_sharev2_search(requested_name=None)
5 changes: 1 addition & 4 deletions trove/views/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,7 @@ def _parse_request(request: http.HttpRequest, search_params_dataclass):
_search_params = search_params_dataclass.from_querystring(
request.META['QUERY_STRING'],
)
_index_strategy_name = _search_params.index_strategy_name
if not _index_strategy_name:
_index_strategy_name = 'trove_indexcard' # TODO: default setting?
_specific_index = IndexStrategy.get_for_searching(_index_strategy_name)
_specific_index = IndexStrategy.get_for_trove_search(_search_params.index_strategy_name)
# TODO: 404 for unknown strategy
_search_gathering = trovesearch_by_indexstrategy.new_gathering({
'search_params': _search_params,
Expand Down

0 comments on commit 8d16765

Please sign in to comment.