Repository: scrapy/scrapy

Issue:
Deprecate the `spider` argument to `Downloader._get_slot_key()`
The `spider` argument is not used inside the method since 2012, but we can't remove it as external code calls it (either because it subclasses `Downloader` or because it wants the slot name).

Actually maybe we want to promote it to a public method for the second reason? Not sure how often it's needed but see e.g. https://github.com/scrapy-plugins/scrapy-zyte-api/blob/a2284c8cdf157ef6d36c1cc413933761c5ed792b/scrapy_zyte_api/_middlewares.py#L32

Discussion:
kumar-sanchay: @wRAR done with the changes you suggested
kumar-sanchay: > It would be nice to have a test that confirms that the old method continues to work as usual but with a warning. But 👍 from me after the release notes issue is handled.

Do you believe testing would be beneficial in this case, considering that the deprecated method internally returning new public method ? 
Gallaecio: > Do you believe testing would be beneficial in this case, considering that the deprecated method internally returning new public method ?

You are right that here it would not add much, I’m OK with not adding it.
Gallaecio: Thank you!

Patch:
diff --git a/docs/news.rst b/docs/news.rst
index fafea0bf8..7db4e59a1 100644
--- a/docs/news.rst
+++ b/docs/news.rst
@@ -3,6 +3,20 @@
 Release notes
 =============
 
+
+.. _release-VERSION:
+
+Scrapy VERSION (YYYY-MM-DD)
+---------------------------
+
+Deprecations
+~~~~~~~~~~~~
+
+-   :func:`scrapy.core.downloader.Downloader._get_slot_key` is now deprecated.
+    Consider using its corresponding public method get_slot_key() instead.
+    (:issue:`6340`)
+
+
 .. _release-2.11.1:
 
 Scrapy 2.11.1 (2024-02-14)
diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py
index 98e1af6fb..0ab3bdb77 100644
--- a/scrapy/core/downloader/__init__.py
+++ b/scrapy/core/downloader/__init__.py
@@ -1,4 +1,5 @@
 import random
+import warnings
 from collections import deque
 from datetime import datetime
 from time import time
@@ -10,6 +11,7 @@ from twisted.internet.defer import Deferred
 from scrapy import Request, Spider, signals
 from scrapy.core.downloader.handlers import DownloadHandlers
 from scrapy.core.downloader.middleware import DownloaderMiddlewareManager
+from scrapy.exceptions import ScrapyDeprecationWarning
 from scrapy.http import Response
 from scrapy.resolver import dnscache
 from scrapy.settings import BaseSettings
@@ -125,7 +127,7 @@ class Downloader:
         return len(self.active) >= self.total_concurrency
 
     def _get_slot(self, request: Request, spider: Spider) -> Tuple[str, Slot]:
-        key = self._get_slot_key(request, spider)
+        key = self.get_slot_key(request)
         if key not in self.slots:
             slot_settings = self.per_slot_settings.get(key, {})
             conc = (
@@ -143,7 +145,7 @@ class Downloader:
 
         return key, self.slots[key]
 
-    def _get_slot_key(self, request: Request, spider: Optional[Spider]) -> str:
+    def get_slot_key(self, request: Request) -> str:
         if self.DOWNLOAD_SLOT in request.meta:
             return cast(str, request.meta[self.DOWNLOAD_SLOT])
 
@@ -153,6 +155,14 @@ class Downloader:
 
         return key
 
+    def _get_slot_key(self, request: Request, spider: Optional[Spider]) -> str:
+        warnings.warn(
+            "Use of this protected method is deprecated. Consider using its corresponding public method get_slot_key() instead.",
+            ScrapyDeprecationWarning,
+            stacklevel=2,
+        )
+        return self.get_slot_key(request)
+
     def _enqueue_request(self, request: Request, spider: Spider) -> Deferred:
         key, slot = self._get_slot(request, spider)
         request.meta[self.DOWNLOAD_SLOT] = key
diff --git a/scrapy/pqueues.py b/scrapy/pqueues.py
index 773825c5e..58a47ef0f 100644
--- a/scrapy/pqueues.py
+++ b/scrapy/pqueues.py
@@ -180,7 +180,7 @@ class DownloaderInterface:
         return [(self._active_downloads(slot), slot) for slot in possible_slots]
 
     def get_slot_key(self, request: Request) -> str:
-        return self.downloader._get_slot_key(request, None)
+        return self.downloader.get_slot_key(request)
 
     def _active_downloads(self, slot: str) -> int:
         """Return a number of requests in a Downloader for a given slot"""


Test patch:
diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py
index 37099dae6..02b50baa3 100644
--- a/tests/test_scheduler.py
+++ b/tests/test_scheduler.py
@@ -25,7 +25,7 @@ class MockDownloader:
     def __init__(self):
         self.slots = {}
 
-    def _get_slot_key(self, request, spider):
+    def get_slot_key(self, request):
         if Downloader.DOWNLOAD_SLOT in request.meta:
             return request.meta[Downloader.DOWNLOAD_SLOT]
 
@@ -273,14 +273,14 @@ class DownloaderAwareSchedulerTestMixin:
         while self.scheduler.has_pending_requests():
             request = self.scheduler.next_request()
             # pylint: disable=protected-access
-            slot = downloader._get_slot_key(request, None)
+            slot = downloader.get_slot_key(request)
             dequeued_slots.append(slot)
             downloader.increment(slot)
             requests.append(request)
 
         for request in requests:
             # pylint: disable=protected-access
-            slot = downloader._get_slot_key(request, None)
+            slot = downloader.get_slot_key(request)
             downloader.decrement(slot)
 
         self.assertTrue(