From 9ef9d38b90d54109c104474fb66134501f1a60a6 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Fri, 10 Apr 2026 07:25:26 -0400 Subject: [PATCH] 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. --- scripts/lib/http.py | 9 +++++ scripts/lib/reddit.py | 82 +++++++------------------------------------ tests/test_http_v3.py | 63 +++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 70 deletions(-) diff --git a/scripts/lib/http.py b/scripts/lib/http.py index b626ac1..5d4898d 100644 --- a/scripts/lib/http.py +++ b/scripts/lib/http.py @@ -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') diff --git a/scripts/lib/reddit.py b/scripts/lib/reddit.py index fc438d8..14a242e 100644 --- a/scripts/lib/reddit.py +++ b/scripts/lib/reddit.py @@ -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) - return data.get("posts", data.get("data", [])) - except http.HTTPError as e: - if e.status_code and 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( + data = http.get( f"{SCRAPECREATORS_BASE}/search", - params={"query": query, "sort": sort, "timeframe": timeframe}, headers=_sc_headers(token), + params={"query": query, "sort": sort, "timeframe": timeframe}, timeout=30, + retries=2, ) - 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) + except http.HTTPError as e: + if e.status_code in (401, 403): + raise _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}") diff --git a/tests/test_http_v3.py b/tests/test_http_v3.py index 321f4eb..dd495e7 100644 --- a/tests/test_http_v3.py +++ b/tests/test_http_v3.py @@ -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)