respond to group review

ARCHITECTURE.md

@@ -103,6 +103,11 @@ Multiple records which describe the same item/object are grouped by a
 the source repository. In most outward-facing views, default to showing only
 the most recent record for each suid.
 
+### Conventions
+(an incomplete list)
+
+- functions prefixed `pls_` ("please") are a request for something to happen
+
 ## Why this?
 inspired by [this writeup](https://matklad.github.io/2021/02/06/ARCHITECTURE.md.html)
 and [this example architecture document](https://github.com/rust-analyzer/rust-analyzer/blob/d7c99931d05e3723d878bea5dc26766791fa4e69/docs/dev/architecture.md)

share/search/index_strategy/__init__.py

@@ -42,6 +42,11 @@ class _AvailableStrategies(enum.Enum):
         trovesearch_denorm = TrovesearchDenormIndexStrategy('trovesearch_denorm')
 
 
+if __debug__:
+    for _strategy_enum in _AvailableStrategies:
+        assert _strategy_enum.name == _strategy_enum.value.strategy_name, 'expected _AvailableStrategies enum name to match strategy name'
+
+
 ###
 # module public interface
 

share/search/index_strategy/_base.py

@@ -150,6 +150,7 @@ def pls_start_backfill(self):
 
     def pls_mark_backfill_complete(self):
         self.get_or_create_backfill().pls_mark_complete()
+        self.pls_refresh()  # explicit refresh after backfill
 
     def pls_check_exists(self) -> bool:
         return all(
@@ -309,10 +310,6 @@ def pls_start_keeping_live(self):
         def pls_stop_keeping_live(self):
             raise NotImplementedError
 
-        @abc.abstractmethod
-        def is_kept_live(self) -> bool:
-            raise NotImplementedError
-
         def pls_get_mappings(self) -> dict:
             raise NotImplementedError
 

share/search/index_strategy/elastic8.py

@@ -256,13 +256,6 @@ def pls_handle_search__passthru(self, request_body=None, request_queryparams=Non
             params=(request_queryparams or {}),
         )
 
-    # override from IndexStrategy
-    def pls_mark_backfill_complete(self):
-        super().pls_mark_backfill_complete()
-        # explicit refresh after bulk operation
-        for _index in self.each_subnamed_index():
-            _index.pls_refresh()
-
     # override from IndexStrategy
     def pls_refresh(self):
         super().pls_refresh()  # refreshes each index
@@ -469,13 +462,6 @@ def pls_stop_keeping_live(self):
             )
             logger.warning('%r: no longer kept live', self)
 
-        # abstract method from IndexStrategy.SpecificIndex
-        def is_kept_live(self) -> bool:
-            _kept_live = self.index_strategy._get_indexnames_for_alias(
-                self.index_strategy._alias_for_keeping_live,
-            )
-            return (self.full_index_name in _kept_live)
-
         def pls_get_mappings(self):
             return self.index_strategy.es8_client.indices.get_mapping(index=self.full_index_name).body
 

share/search/index_strategy/sharev2_elastic5.py

@@ -367,10 +367,6 @@ def pls_create(self):
         def pls_start_keeping_live(self):
             pass  # there is just the one index, always kept live
 
-        # abstract method from IndexStrategy.SpecificIndex
-        def is_kept_live(self) -> bool:
-            return True  # there is just the one index, always kept live
-
         # abstract method from IndexStrategy.SpecificIndex
         def pls_stop_keeping_live(self):
             raise exceptions.IndexStrategyError(

tests/share/search/index_strategy/_with_real_services.py

@@ -85,36 +85,36 @@ def _assert_happypath_with_daemon(self, messages_chunk, expected_doc_count):
             assert False, 'checked and waited but the daemon did not do the thing'
 
     def _assert_setup_happypath(self):
-        # initial
+        # initial (no indexes exist)
         for _index in self.index_strategy.each_subnamed_index():
             assert not _index.pls_check_exists()
             index_status = _index.pls_get_status()
             assert not index_status.creation_date
             assert not index_status.is_kept_live
             assert not index_status.is_default_for_searching
             assert not index_status.doc_count
+        # create each index
         for _index in self.index_strategy.each_subnamed_index():
             _index.pls_create()
-            # create index
-            assert _index.pls_check_exists()
+            assert _index.pls_check_exists()  # new!
             index_status = _index.pls_get_status()
-            assert index_status.creation_date
+            assert index_status.creation_date  # new!
             assert not index_status.is_kept_live
             assert not index_status.is_default_for_searching
             assert not index_status.doc_count
-        # keep index live (with ingested updates)
+        # start keeping each index live (with ingested updates)
         self.index_strategy.pls_start_keeping_live()
         for _index in self.index_strategy.each_subnamed_index():
             index_status = _index.pls_get_status()
             assert index_status.creation_date
-            assert index_status.is_kept_live
+            assert index_status.is_kept_live  # new!
             assert not index_status.is_default_for_searching
             assert not index_status.doc_count
-        # default index for searching
+        # make this version of the strategy the default for searching
         self.index_strategy.pls_make_default_for_searching()
         for _index in self.index_strategy.each_subnamed_index():
             index_status = _index.pls_get_status()
             assert index_status.creation_date
             assert index_status.is_kept_live
-            assert index_status.is_default_for_searching
-            assert not index_status.doc_count
+            assert index_status.is_default_for_searching  # new!
+            assert not index_status.doc_count  # (still empty)