refactor(reddit): migrate to http.get(params=...) helper
Added params kwarg to http.request()/http.get() that urlencodes a dict into the query string. None values are dropped, ints and bools are stringified, and params append correctly if the URL already has a query string. Migrated reddit.py to use this helper for all three ScrapeCreators call sites (global search, subreddit search, post comments). Deleted the try/import requests/except ImportError fallback and the paired if not _requests: / else: branches. Six new http tests cover the params-encoding behavior. Net: reddit.py -70 lines. Behavior is identical - the existing http.py urllib implementation already had retry logic, 429 handling, and HTTPError types that are strictly better than the ad-hoc requests branches we deleted. 99 reddit tests pass. Live smoke test on a real ScrapeCreators run returned 12 threads with the same engagement data as before.
This commit is contained in:
@@ -38,6 +38,7 @@ def request(
|
||||
url: str,
|
||||
headers: Optional[Dict[str, str]] = None,
|
||||
json_data: Optional[Dict[str, Any]] = None,
|
||||
params: Optional[Dict[str, Any]] = None,
|
||||
timeout: int = DEFAULT_TIMEOUT,
|
||||
retries: int = MAX_RETRIES,
|
||||
max_429_retries: int = MAX_429_RETRIES,
|
||||
@@ -50,6 +51,8 @@ def request(
|
||||
url: Request URL
|
||||
headers: Optional headers dict
|
||||
json_data: Optional JSON body (for POST)
|
||||
params: Optional query-string params. Values are stringified. None values
|
||||
are dropped. If ``url`` already has a query string, ``params`` is appended.
|
||||
timeout: Request timeout in seconds
|
||||
retries: Number of retries on failure
|
||||
max_429_retries: Maximum 429 retries before giving up (separate cap)
|
||||
@@ -64,6 +67,12 @@ def request(
|
||||
headers = headers or {}
|
||||
headers.setdefault("User-Agent", USER_AGENT)
|
||||
|
||||
if params:
|
||||
filtered = {k: str(v) for k, v in params.items() if v is not None}
|
||||
if filtered:
|
||||
separator = "&" if ("?" in url) else "?"
|
||||
url = f"{url}{separator}{urlencode(filtered)}"
|
||||
|
||||
data = None
|
||||
if json_data is not None:
|
||||
data = json.dumps(json_data).encode('utf-8')
|
||||
|
||||
+14
-72
@@ -15,12 +15,6 @@ from concurrent.futures import ThreadPoolExecutor, as_completed, wait as futures
|
||||
from datetime import datetime, timezone
|
||||
from typing import Any, Dict, List, Optional, Set
|
||||
|
||||
try:
|
||||
import requests as _requests
|
||||
except ImportError:
|
||||
_requests = None
|
||||
|
||||
|
||||
def _first_of(*values, default=None):
|
||||
"""Return first value that is not None."""
|
||||
for v in values:
|
||||
@@ -350,39 +344,18 @@ def _global_search(
|
||||
Returns:
|
||||
List of post dicts
|
||||
"""
|
||||
if not _requests:
|
||||
_log("requests library not installed, falling back to urllib")
|
||||
# Use stdlib http module as fallback
|
||||
try:
|
||||
from urllib.parse import urlencode
|
||||
params = urlencode({"query": query, "sort": sort, "timeframe": timeframe})
|
||||
url = f"{SCRAPECREATORS_BASE}/search?{params}"
|
||||
headers = _sc_headers(token)
|
||||
headers["User-Agent"] = http.USER_AGENT
|
||||
data = http.get(url, headers=headers, timeout=30, retries=2)
|
||||
data = http.get(
|
||||
f"{SCRAPECREATORS_BASE}/search",
|
||||
headers=_sc_headers(token),
|
||||
params={"query": query, "sort": sort, "timeframe": timeframe},
|
||||
timeout=30,
|
||||
retries=2,
|
||||
)
|
||||
return data.get("posts", data.get("data", []))
|
||||
except http.HTTPError as e:
|
||||
if e.status_code and e.status_code in (401, 403):
|
||||
if e.status_code in (401, 403):
|
||||
raise
|
||||
_log(f"Global search error (urllib): {e}")
|
||||
return []
|
||||
except Exception as e:
|
||||
_log(f"Global search error (urllib): {e}")
|
||||
return []
|
||||
|
||||
try:
|
||||
resp = _requests.get(
|
||||
f"{SCRAPECREATORS_BASE}/search",
|
||||
params={"query": query, "sort": sort, "timeframe": timeframe},
|
||||
headers=_sc_headers(token),
|
||||
timeout=30,
|
||||
)
|
||||
resp.raise_for_status()
|
||||
data = resp.json()
|
||||
return data.get("posts", data.get("data", []))
|
||||
except _requests.exceptions.HTTPError as e:
|
||||
if e.response is not None and e.response.status_code in (401, 403):
|
||||
raise http.HTTPError(f"Auth error: {e}", e.response.status_code)
|
||||
_log(f"Global search error: {e}")
|
||||
return []
|
||||
except Exception as e:
|
||||
@@ -409,36 +382,19 @@ def _subreddit_search(
|
||||
Returns:
|
||||
List of post dicts
|
||||
"""
|
||||
if not _requests:
|
||||
try:
|
||||
from urllib.parse import urlencode
|
||||
params = urlencode({
|
||||
"subreddit": subreddit, "query": query,
|
||||
"sort": sort, "timeframe": timeframe,
|
||||
})
|
||||
url = f"{SCRAPECREATORS_BASE}/subreddit/search?{params}"
|
||||
headers = _sc_headers(token)
|
||||
headers["User-Agent"] = http.USER_AGENT
|
||||
data = http.get(url, headers=headers, timeout=30, retries=2)
|
||||
return data.get("posts", data.get("data", []))
|
||||
except Exception as e:
|
||||
_log(f"Subreddit search error (urllib) for r/{subreddit}: {e}")
|
||||
return []
|
||||
|
||||
try:
|
||||
resp = _requests.get(
|
||||
data = http.get(
|
||||
f"{SCRAPECREATORS_BASE}/subreddit/search",
|
||||
headers=_sc_headers(token),
|
||||
params={
|
||||
"subreddit": subreddit,
|
||||
"query": query,
|
||||
"sort": sort,
|
||||
"timeframe": timeframe,
|
||||
},
|
||||
headers=_sc_headers(token),
|
||||
timeout=30,
|
||||
retries=2,
|
||||
)
|
||||
resp.raise_for_status()
|
||||
data = resp.json()
|
||||
return data.get("posts", data.get("data", []))
|
||||
except Exception as e:
|
||||
_log(f"Subreddit search error for r/{subreddit}: {e}")
|
||||
@@ -458,28 +414,14 @@ def fetch_post_comments(
|
||||
Returns:
|
||||
List of comment dicts with score, author, body, etc.
|
||||
"""
|
||||
if not _requests:
|
||||
try:
|
||||
from urllib.parse import urlencode
|
||||
params = urlencode({"url": url})
|
||||
api_url = f"{SCRAPECREATORS_BASE}/post/comments?{params}"
|
||||
headers = _sc_headers(token)
|
||||
headers["User-Agent"] = http.USER_AGENT
|
||||
data = http.get(api_url, headers=headers, timeout=30, retries=2)
|
||||
return data.get("comments", data.get("data", []))
|
||||
except Exception as e:
|
||||
_log(f"Comment fetch error (urllib): {e}")
|
||||
return []
|
||||
|
||||
try:
|
||||
resp = _requests.get(
|
||||
data = http.get(
|
||||
f"{SCRAPECREATORS_BASE}/post/comments",
|
||||
params={"url": url},
|
||||
headers=_sc_headers(token),
|
||||
params={"url": url},
|
||||
timeout=30,
|
||||
retries=2,
|
||||
)
|
||||
resp.raise_for_status()
|
||||
data = resp.json()
|
||||
return data.get("comments", data.get("data", []))
|
||||
except Exception as e:
|
||||
_log(f"Comment fetch error: {e}")
|
||||
|
||||
@@ -41,3 +41,66 @@ class Test429RetryLimit(unittest.TestCase):
|
||||
http.request("GET", "http://example.com", retries=3)
|
||||
|
||||
self.assertEqual(mock_urlopen.call_count, 3)
|
||||
|
||||
|
||||
def _mock_response(body: str = '{"ok": true}', status: int = 200):
|
||||
resp = MagicMock()
|
||||
resp.__enter__ = MagicMock(return_value=resp)
|
||||
resp.__exit__ = MagicMock(return_value=False)
|
||||
resp.read.return_value = body.encode("utf-8")
|
||||
resp.status = status
|
||||
return resp
|
||||
|
||||
|
||||
class TestParamsEncoding(unittest.TestCase):
|
||||
"""request() should urlencode the params dict into the URL."""
|
||||
|
||||
def _sent_url(self, mock_urlopen) -> str:
|
||||
request_arg = mock_urlopen.call_args[0][0]
|
||||
return request_arg.full_url
|
||||
|
||||
@patch("lib.http.urllib.request.urlopen")
|
||||
def test_params_appended_to_url(self, mock_urlopen):
|
||||
mock_urlopen.return_value = _mock_response()
|
||||
http.get("https://api.example.com/search", params={"q": "test", "limit": 10})
|
||||
sent_url = self._sent_url(mock_urlopen)
|
||||
self.assertIn("q=test", sent_url)
|
||||
self.assertIn("limit=10", sent_url)
|
||||
|
||||
@patch("lib.http.urllib.request.urlopen")
|
||||
def test_params_appended_with_existing_query_string(self, mock_urlopen):
|
||||
mock_urlopen.return_value = _mock_response()
|
||||
http.get("https://api.example.com/search?api_key=secret", params={"q": "test"})
|
||||
sent_url = self._sent_url(mock_urlopen)
|
||||
self.assertTrue(sent_url.startswith("https://api.example.com/search?api_key=secret&"))
|
||||
self.assertIn("q=test", sent_url)
|
||||
|
||||
@patch("lib.http.urllib.request.urlopen")
|
||||
def test_none_values_dropped(self, mock_urlopen):
|
||||
mock_urlopen.return_value = _mock_response()
|
||||
http.get("https://api.example.com/search", params={"q": "test", "filter": None})
|
||||
sent_url = self._sent_url(mock_urlopen)
|
||||
self.assertIn("q=test", sent_url)
|
||||
self.assertNotIn("filter", sent_url)
|
||||
|
||||
@patch("lib.http.urllib.request.urlopen")
|
||||
def test_empty_params_leaves_url_unchanged(self, mock_urlopen):
|
||||
mock_urlopen.return_value = _mock_response()
|
||||
http.get("https://api.example.com/search", params={})
|
||||
sent_url = self._sent_url(mock_urlopen)
|
||||
self.assertEqual(sent_url, "https://api.example.com/search")
|
||||
|
||||
@patch("lib.http.urllib.request.urlopen")
|
||||
def test_no_params_kwarg_leaves_url_unchanged(self, mock_urlopen):
|
||||
mock_urlopen.return_value = _mock_response()
|
||||
http.get("https://api.example.com/search")
|
||||
sent_url = self._sent_url(mock_urlopen)
|
||||
self.assertEqual(sent_url, "https://api.example.com/search")
|
||||
|
||||
@patch("lib.http.urllib.request.urlopen")
|
||||
def test_int_and_bool_params_stringified(self, mock_urlopen):
|
||||
mock_urlopen.return_value = _mock_response()
|
||||
http.get("https://api.example.com/search", params={"count": 25, "raw": True})
|
||||
sent_url = self._sent_url(mock_urlopen)
|
||||
self.assertIn("count=25", sent_url)
|
||||
self.assertIn("raw=True", sent_url)
|
||||
|
||||
Reference in New Issue
Block a user