Merge pull request #207 from iliaal/refactor/reddit-http-helper
refactor(reddit): migrate to http.get(params=...) helper
This commit is contained in:
@@ -38,6 +38,7 @@ def request(
|
|||||||
url: str,
|
url: str,
|
||||||
headers: Optional[Dict[str, str]] = None,
|
headers: Optional[Dict[str, str]] = None,
|
||||||
json_data: Optional[Dict[str, Any]] = None,
|
json_data: Optional[Dict[str, Any]] = None,
|
||||||
|
params: Optional[Dict[str, Any]] = None,
|
||||||
timeout: int = DEFAULT_TIMEOUT,
|
timeout: int = DEFAULT_TIMEOUT,
|
||||||
retries: int = MAX_RETRIES,
|
retries: int = MAX_RETRIES,
|
||||||
max_429_retries: int = MAX_429_RETRIES,
|
max_429_retries: int = MAX_429_RETRIES,
|
||||||
@@ -50,6 +51,8 @@ def request(
|
|||||||
url: Request URL
|
url: Request URL
|
||||||
headers: Optional headers dict
|
headers: Optional headers dict
|
||||||
json_data: Optional JSON body (for POST)
|
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
|
timeout: Request timeout in seconds
|
||||||
retries: Number of retries on failure
|
retries: Number of retries on failure
|
||||||
max_429_retries: Maximum 429 retries before giving up (separate cap)
|
max_429_retries: Maximum 429 retries before giving up (separate cap)
|
||||||
@@ -64,6 +67,12 @@ def request(
|
|||||||
headers = headers or {}
|
headers = headers or {}
|
||||||
headers.setdefault("User-Agent", USER_AGENT)
|
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
|
data = None
|
||||||
if json_data is not None:
|
if json_data is not None:
|
||||||
data = json.dumps(json_data).encode('utf-8')
|
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 datetime import datetime, timezone
|
||||||
from typing import Any, Dict, List, Optional, Set
|
from typing import Any, Dict, List, Optional, Set
|
||||||
|
|
||||||
try:
|
|
||||||
import requests as _requests
|
|
||||||
except ImportError:
|
|
||||||
_requests = None
|
|
||||||
|
|
||||||
|
|
||||||
def _first_of(*values, default=None):
|
def _first_of(*values, default=None):
|
||||||
"""Return first value that is not None."""
|
"""Return first value that is not None."""
|
||||||
for v in values:
|
for v in values:
|
||||||
@@ -350,39 +344,18 @@ def _global_search(
|
|||||||
Returns:
|
Returns:
|
||||||
List of post dicts
|
List of post dicts
|
||||||
"""
|
"""
|
||||||
if not _requests:
|
|
||||||
_log("requests library not installed, falling back to urllib")
|
|
||||||
# Use stdlib http module as fallback
|
|
||||||
try:
|
try:
|
||||||
from urllib.parse import urlencode
|
data = http.get(
|
||||||
params = urlencode({"query": query, "sort": sort, "timeframe": timeframe})
|
f"{SCRAPECREATORS_BASE}/search",
|
||||||
url = f"{SCRAPECREATORS_BASE}/search?{params}"
|
headers=_sc_headers(token),
|
||||||
headers = _sc_headers(token)
|
params={"query": query, "sort": sort, "timeframe": timeframe},
|
||||||
headers["User-Agent"] = http.USER_AGENT
|
timeout=30,
|
||||||
data = http.get(url, headers=headers, timeout=30, retries=2)
|
retries=2,
|
||||||
|
)
|
||||||
return data.get("posts", data.get("data", []))
|
return data.get("posts", data.get("data", []))
|
||||||
except http.HTTPError as e:
|
except http.HTTPError as e:
|
||||||
if e.status_code and e.status_code in (401, 403):
|
if e.status_code in (401, 403):
|
||||||
raise
|
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}")
|
_log(f"Global search error: {e}")
|
||||||
return []
|
return []
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
@@ -409,36 +382,19 @@ def _subreddit_search(
|
|||||||
Returns:
|
Returns:
|
||||||
List of post dicts
|
List of post dicts
|
||||||
"""
|
"""
|
||||||
if not _requests:
|
|
||||||
try:
|
try:
|
||||||
from urllib.parse import urlencode
|
data = http.get(
|
||||||
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(
|
|
||||||
f"{SCRAPECREATORS_BASE}/subreddit/search",
|
f"{SCRAPECREATORS_BASE}/subreddit/search",
|
||||||
|
headers=_sc_headers(token),
|
||||||
params={
|
params={
|
||||||
"subreddit": subreddit,
|
"subreddit": subreddit,
|
||||||
"query": query,
|
"query": query,
|
||||||
"sort": sort,
|
"sort": sort,
|
||||||
"timeframe": timeframe,
|
"timeframe": timeframe,
|
||||||
},
|
},
|
||||||
headers=_sc_headers(token),
|
|
||||||
timeout=30,
|
timeout=30,
|
||||||
|
retries=2,
|
||||||
)
|
)
|
||||||
resp.raise_for_status()
|
|
||||||
data = resp.json()
|
|
||||||
return data.get("posts", data.get("data", []))
|
return data.get("posts", data.get("data", []))
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
_log(f"Subreddit search error for r/{subreddit}: {e}")
|
_log(f"Subreddit search error for r/{subreddit}: {e}")
|
||||||
@@ -458,28 +414,14 @@ def fetch_post_comments(
|
|||||||
Returns:
|
Returns:
|
||||||
List of comment dicts with score, author, body, etc.
|
List of comment dicts with score, author, body, etc.
|
||||||
"""
|
"""
|
||||||
if not _requests:
|
|
||||||
try:
|
try:
|
||||||
from urllib.parse import urlencode
|
data = http.get(
|
||||||
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(
|
|
||||||
f"{SCRAPECREATORS_BASE}/post/comments",
|
f"{SCRAPECREATORS_BASE}/post/comments",
|
||||||
params={"url": url},
|
|
||||||
headers=_sc_headers(token),
|
headers=_sc_headers(token),
|
||||||
|
params={"url": url},
|
||||||
timeout=30,
|
timeout=30,
|
||||||
|
retries=2,
|
||||||
)
|
)
|
||||||
resp.raise_for_status()
|
|
||||||
data = resp.json()
|
|
||||||
return data.get("comments", data.get("data", []))
|
return data.get("comments", data.get("data", []))
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
_log(f"Comment fetch error: {e}")
|
_log(f"Comment fetch error: {e}")
|
||||||
|
|||||||
@@ -41,3 +41,66 @@ class Test429RetryLimit(unittest.TestCase):
|
|||||||
http.request("GET", "http://example.com", retries=3)
|
http.request("GET", "http://example.com", retries=3)
|
||||||
|
|
||||||
self.assertEqual(mock_urlopen.call_count, 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