Merge pull request #1795 from Numerlor/doc-repetitive-outdated-warn

Prevent repetitive stale inventory warnings from erroneously generated inventories

Author: wookie184

bot/exts/info/doc/_batch_parser.py

@@ -17,13 +17,16 @@
 
 from . import _cog, doc_cache
 from ._parsing import get_symbol_markdown
+from ._redis_cache import StaleItemCounter
 
 log = get_logger(__name__)
 
 
 class StaleInventoryNotifier:
     """Handle sending notifications about stale inventories through `DocItem`s to dev log."""
 
+    symbol_counter = StaleItemCounter()
+
     def __init__(self):
         self._init_task = scheduling.create_task(
             self._init_channel(),
@@ -40,13 +43,16 @@ async def _init_channel(self) -> None:
     async def send_warning(self, doc_item: _cog.DocItem) -> None:
         """Send a warning to dev log if one wasn't already sent for `item`'s url."""
         if doc_item.url not in self._warned_urls:
-            self._warned_urls.add(doc_item.url)
-            await self._init_task
-            embed = discord.Embed(
-                description=f"Doc item `{doc_item.symbol_id=}` present in loaded documentation inventories "
-                            f"not found on [site]({doc_item.url}), inventories may need to be refreshed."
-            )
-            await self._dev_log.send(embed=embed)
+            # Only warn if the item got less than 3 warnings
+            # or if it has been more than 3 weeks since the last warning
+            if await self.symbol_counter.increment_for(doc_item) < 3:
+                self._warned_urls.add(doc_item.url)
+                await self._init_task
+                embed = discord.Embed(
+                    description=f"Doc item `{doc_item.symbol_id=}` present in loaded documentation inventories "
+                                f"not found on [site]({doc_item.url}), inventories may need to be refreshed."
+                )
+                await self._dev_log.send(embed=embed)
 
 
 class QueueItem(NamedTuple):
@@ -103,7 +109,7 @@ async def get_markdown(self, doc_item: _cog.DocItem) -> Optional[str]:
         if doc_item not in self._item_futures and doc_item not in self._queue:
             self._item_futures[doc_item].user_requested = True
 
-            async with bot.instance.http_session.get(doc_item.url) as response:
+            async with bot.instance.http_session.get(doc_item.url, raise_for_status=True) as response:
                 soup = await bot.instance.loop.run_in_executor(
                     None,
                     BeautifulSoup,

bot/exts/info/doc/_cog.py

@@ -464,6 +464,7 @@ async def clear_cache_command(
     ) -> None:
         """Clear the persistent redis cache for `package`."""
         if await doc_cache.delete(package_name):
+            await self.item_fetcher.stale_inventory_notifier.symbol_counter.delete()
             await ctx.send(f"Successfully cleared the cache for `{package_name}`.")
         else:
             await ctx.send("No keys matching the package found.")

bot/exts/info/doc/_redis_cache.py

@@ -25,8 +25,7 @@ async def set(self, item: DocItem, value: str) -> None:
 
         All keys from a single page are stored together, expiring a week after the first set.
         """
-        url_key = remove_suffix(item.relative_url_path, ".html")
-        redis_key = f"{self.namespace}:{item.package}:{url_key}"
+        redis_key = f"{self.namespace}:{item_key(item)}"
         needs_expire = False
 
         with await self._get_pool_connection() as connection:
@@ -44,10 +43,36 @@ async def set(self, item: DocItem, value: str) -> None:
     @namespace_lock
     async def get(self, item: DocItem) -> Optional[str]:
         """Return the Markdown content of the symbol `item` if it exists."""
-        url_key = remove_suffix(item.relative_url_path, ".html")
+        with await self._get_pool_connection() as connection:
+            return await connection.hget(f"{self.namespace}:{item_key(item)}", item.symbol_id, encoding="utf8")
 
+    @namespace_lock
+    async def delete(self, package: str) -> bool:
+        """Remove all values for `package`; return True if at least one key was deleted, False otherwise."""
+        with await self._get_pool_connection() as connection:
+            package_keys = [
+                package_key async for package_key in connection.iscan(match=f"{self.namespace}:{package}:*")
+            ]
+            if package_keys:
+                await connection.delete(*package_keys)
+                return True
+            return False
+
+
+class StaleItemCounter(RedisObject):
+    """Manage increment counters for stale `DocItem`s."""
+
+    @namespace_lock
+    async def increment_for(self, item: DocItem) -> int:
+        """
+        Increment the counter for `item` by 1, set it to expire in 3 weeks and return the new value.
+
+        If the counter didn't exist, initialize it with 1.
+        """
+        key = f"{self.namespace}:{item_key(item)}:{item.symbol_id}"
         with await self._get_pool_connection() as connection:
-            return await connection.hget(f"{self.namespace}:{item.package}:{url_key}", item.symbol_id, encoding="utf8")
+            await connection.expire(key, WEEK_SECONDS * 3)
+            return int(await connection.incr(key))
 
     @namespace_lock
     async def delete(self, package: str) -> bool:
@@ -62,10 +87,6 @@ async def delete(self, package: str) -> bool:
             return False
 
 
-def remove_suffix(string: str, suffix: str) -> str:
-    """Remove `suffix` from end of `string`."""
-    # TODO replace usages with str.removesuffix on 3.9
-    if string.endswith(suffix):
-        return string[:-len(suffix)]
-    else:
-        return string
+def item_key(item: DocItem) -> str:
+    """Get the redis redis key string from `item`."""
+    return f"{item.package}:{item.relative_url_path.removesuffix('.html')}"