fix(dataflows): search all subreddits in one Reddit request

- anonymous RSS allows about one request per minute per IP, so fetching each
  subreddit separately hit a 429 and a 60s back-off on nearly every run
- search the subreddits as one combined feed at Reddit's full page size and
  group posts by the subreddit each entry names; each subreddit keeps its own
  limit, and a full page is not taken as evidence of absence
- drop the unused JSON search path, still blocked with a 403, and the
  per-subreddit pacing; arguments after subreddits are keyword-only
- the sentiment prompt no longer asks for vote and comment counts, which the
  RSS feed does not carry
This commit is contained in:
Yijia-Xiao
2026-09-14 22:07:42 +00:00
parent 2e38b47dca
commit 241638da68
4 changed files with 172 additions and 263 deletions

View File

@@ -1,5 +1,5 @@
"""Tests for the RSS-first Reddit fetcher, its 429 backoff, the opt-in JSON
path's degradation (#862), and chunked-transfer error handling (#1024)."""
"""Tests for the Reddit RSS fetcher: one combined request, its 429 backoff, and
chunked-transfer error handling (#1024)."""
from __future__ import annotations
@@ -80,48 +80,15 @@ class TestRssParsing:
posts = reddit._fetch_subreddit_rss("NVDA", "stocks", limit=5, timeout=5.0)
assert len(posts) == 2
assert posts[0]["title"] == "NVDA earnings beat, stock pops"
assert posts[0]["source"] == "rss"
assert posts[0]["score"] is None
assert posts[0]["num_comments"] is None
assert posts[0]["created_utc"] > 0
assert "datacenter unit" in posts[0]["selftext"]
assert posts[0]["subreddit"] == "stocks"
def test_malformed_xml_reports_unavailable(self):
with patch.object(reddit, "urlopen", return_value=_resp(lambda: b"<<not xml>>")):
assert reddit._fetch_subreddit_rss("NVDA", "stocks", 5, 5.0) is None
@pytest.mark.unit
class TestFetchSubredditIsRssFirst:
"""The default per-subreddit fetch goes straight to RSS — it must not hit
the WAF-blocked JSON endpoint, which only burned rate-limit budget."""
def test_delegates_to_rss_without_touching_json(self):
sentinel = [{"title": "x", "source": "rss", "score": None,
"num_comments": None, "created_utc": None, "selftext": ""}]
with patch.object(reddit, "_fetch_subreddit_rss", return_value=sentinel) as rss, \
patch.object(reddit, "urlopen",
side_effect=AssertionError("JSON endpoint must not be called")):
out = reddit._fetch_subreddit("NVDA", "stocks", 5, 5.0)
rss.assert_called_once()
assert out is sentinel
@pytest.mark.unit
class TestJsonPathFallsBackToRss:
"""The opt-in JSON path still degrades to RSS on a 403 (kept for #862)."""
def test_403_triggers_rss(self):
err = HTTPError("url", 403, "Blocked", {}, None)
rss_posts = [{"title": "x", "source": "rss", "score": None,
"num_comments": None, "created_utc": None, "selftext": ""}]
with patch.object(reddit, "urlopen", side_effect=err), \
patch.object(reddit, "_fetch_subreddit_rss", return_value=rss_posts) as rss:
out = reddit._fetch_subreddit_json("NVDA", "stocks", 5, 5.0)
rss.assert_called_once()
assert out and out[0]["source"] == "rss"
@pytest.mark.unit
class TestRss429Backoff:
def test_429_then_success_retries_once(self):
@@ -178,12 +145,6 @@ class TestChunkedTransferErrorsHandled:
with patch.object(reddit, "urlopen", return_value=_raise(http.client.IncompleteRead(b""))):
assert reddit._fetch_subreddit_rss("NVDA", "stocks", 5, 5.0) is None
def test_json_incomplete_read_falls_back_to_rss(self):
with patch.object(reddit, "urlopen", return_value=_raise(http.client.IncompleteRead(b""))), \
patch.object(reddit, "_fetch_subreddit_rss", return_value=[]) as rss:
reddit._fetch_subreddit_json("NVDA", "stocks", 5, 5.0)
rss.assert_called_once()
def test_oversized_rss_feed_is_refused_not_parsed(self):
# A hostile/misbehaving endpoint streaming an unbounded body must not be
# read into memory before parsing; overflow degrades to an empty feed.
@@ -201,25 +162,12 @@ class TestFormatterHandlesRssPosts:
"created_utc": reddit._iso_to_timestamp("2026-05-20T14:30:00Z"),
"selftext": "great quarter", "source": "rss",
}]
with patch.object(reddit, "_fetch_subreddit", return_value=rss_posts):
out = reddit.fetch_reddit_posts("NVDA", subreddits=("stocks",), inter_request_delay=0)
assert "via RSS feed" in out
assert "" not in out # no fake score arrow
with patch.object(reddit, "_fetch_subreddit_rss", return_value=rss_posts):
out = reddit.fetch_reddit_posts("NVDA", subreddits=("stocks",))
assert "" not in out # RSS has no scores; none are invented
assert "NVDA pops" in out
assert "great quarter" in out
def test_json_posts_still_show_counts(self):
json_posts = [{
"title": "NVDA pops", "score": 1234, "num_comments": 56,
"created_utc": reddit._iso_to_timestamp("2026-05-20T14:30:00Z"),
"selftext": "",
}]
with patch.object(reddit, "_fetch_subreddit", return_value=json_posts):
out = reddit.fetch_reddit_posts("NVDA", subreddits=("stocks",), inter_request_delay=0)
assert "1234↑" in out
assert "56c" in out
assert "via RSS" not in out
@pytest.mark.unit
class TestCryptoSearchTerm:
@@ -228,12 +176,12 @@ class TestCryptoSearchTerm:
def _captured_ticker(self, ticker):
seen = {}
def fake_fetch(t, sub, limit, timeout, **kwargs):
def fake_fetch(t, subs, limit, timeout, **kwargs):
seen["ticker"] = t
return []
with patch.object(reddit, "_fetch_subreddit", side_effect=fake_fetch):
reddit.fetch_reddit_posts(ticker, subreddits=("stocks",), inter_request_delay=0)
with patch.object(reddit, "_fetch_subreddit_rss", side_effect=fake_fetch):
reddit.fetch_reddit_posts(ticker, subreddits=("stocks",))
return seen["ticker"]
def test_crypto_pair_searches_base(self):
@@ -244,61 +192,83 @@ class TestCryptoSearchTerm:
@pytest.mark.unit
class TestFailedFetchIsNotSilence:
"""A throttled fetch must not be rendered as "no posts found" (#1295).
class TestOneRequestForAllSubreddits:
"""Reddit's anonymous RSS allows about one request per minute per IP, so a
request per subreddit spent a back-off on nearly every run. One combined
feed (``r/a+b+c``) carries each entry's subreddit, so nothing is lost."""
Returning [] for both a failed request and a genuinely empty search made the
sentiment analyst read rate limiting as real silence ("r/stocks and
r/investing are silent"), which is a signal that was never observed.
"""
def _post(self, sub, title="NVDA pops"):
return {"title": title, "score": None, "num_comments": None,
"created_utc": reddit._iso_to_timestamp("2026-05-20T14:30:00Z"),
"selftext": "", "source": "rss", "subreddit": sub}
_POST = {
"title": "NVDA pops", "score": None, "num_comments": None,
"created_utc": reddit._iso_to_timestamp("2026-05-20T14:30:00Z"),
"selftext": "", "source": "rss",
}
def test_all_subreddits_share_one_request(self):
calls = []
def _run(self, results):
"""Drive fetch_reddit_posts with a per-subreddit result sequence."""
subs = tuple(f"s{i}" for i in range(len(results)))
with patch.object(reddit, "_fetch_subreddit", side_effect=list(results)):
return reddit.fetch_reddit_posts(
"NVDA", subreddits=subs, inter_request_delay=0
)
def record(t, subs, limit, timeout):
calls.append((subs, limit))
return []
def test_failed_subreddit_is_marked_unavailable_not_empty(self):
out = self._run([None, [self._POST]])
assert "unavailable" in out
assert "no posts found" not in out.split("unavailable")[0]
with patch.object(reddit, "_fetch_subreddit_rss", side_effect=record):
reddit.fetch_reddit_posts("NVDA", subreddits=("a", "b", "c"), limit_per_sub=5)
# One full page, so a busy subreddit cannot crowd the others out.
assert calls == [("a+b+c", reddit._FEED_PAGE)]
def test_all_sources_failing_does_not_claim_no_posts(self):
out = self._run([None, None])
def test_posts_are_grouped_back_by_subreddit(self):
posts = [self._post("b", "FROM B"), self._post("a", "FROM A")]
with patch.object(reddit, "_fetch_subreddit_rss", return_value=posts):
out = reddit.fetch_reddit_posts("NVDA", subreddits=("a", "b"))
assert out.index("r/a") < out.index("FROM A") < out.index("r/b") < out.index("FROM B")
def test_failed_request_is_unavailable_not_silence(self):
# #1295: a throttled fetch must not read as "no posts found".
with patch.object(reddit, "_fetch_subreddit_rss", return_value=None):
out = reddit.fetch_reddit_posts("NVDA", subreddits=("a", "b"))
assert "Reddit unavailable" in out
assert "no Reddit posts found" not in out
def test_mixed_failure_and_empty_only_claims_silence_for_searched_subs(self):
# s0 failed, s1 genuinely returned nothing: the "no posts" claim must
# cover only s1, with s0 reported separately as unavailable.
out = self._run([None, []])
assert "r/s1" in out.split("unavailable (fetch failed)")[0]
assert "unavailable (fetch failed): r/s0" in out
def test_genuine_empty_still_reports_no_posts(self):
out = self._run([[], []])
with patch.object(reddit, "_fetch_subreddit_rss", return_value=[]):
out = reddit.fetch_reddit_posts("NVDA", subreddits=("a", "b"))
assert "no Reddit posts found" in out
assert "unavailable" not in out
def test_retry_is_not_spent_again_after_a_failure(self):
# The 60s back-off must be paid at most once per run, so subsequent
# subreddits are fetched with retry disabled rather than stalling.
seen = []
def test_subreddit_with_no_posts_is_listed_when_others_have_some(self):
with patch.object(reddit, "_fetch_subreddit_rss", return_value=[self._post("a")]):
out = reddit.fetch_reddit_posts("NVDA", subreddits=("a", "b"))
assert "r/b: <no posts found" in out
def record(t, sub, limit, timeout, _retry=True):
seen.append(_retry)
return None
with patch.object(reddit, "_fetch_subreddit", side_effect=record):
reddit.fetch_reddit_posts(
"NVDA", subreddits=("a", "b", "c"), inter_request_delay=0
)
assert seen == [True, False, False]
@pytest.mark.unit
def test_posts_from_an_unrequested_or_unnamed_subreddit_are_not_dropped():
posts = [
{"title": "ELSEWHERE", "created_utc": None, "selftext": "", "subreddit": "options"},
{"title": "NO LABEL", "created_utc": None, "selftext": "", "subreddit": ""},
]
with patch.object(reddit, "_fetch_subreddit_rss", return_value=posts):
out = reddit.fetch_reddit_posts("NVDA", subreddits=("a", "b"))
assert "ELSEWHERE" in out and "r/options" in out
assert "NO LABEL" in out
@pytest.mark.unit
def test_each_subreddit_keeps_its_own_quota():
busy = [{"title": f"A{i}", "created_utc": None, "selftext": "", "subreddit": "a"} for i in range(9)]
quiet = [{"title": "B0", "created_utc": None, "selftext": "", "subreddit": "b"}]
with patch.object(reddit, "_fetch_subreddit_rss", return_value=busy + quiet):
out = reddit.fetch_reddit_posts("NVDA", subreddits=("a", "b"), limit_per_sub=3)
assert "A0" in out and "A2" in out and "A3" not in out # capped per subreddit
assert "B0" in out # not crowded out
@pytest.mark.unit
def test_empty_subreddit_on_a_full_page_is_not_called_empty():
# A full page may have cut a quieter subreddit's posts off, so its absence
# from the page is not evidence of no posts.
full = [{"title": f"A{i}", "created_utc": None, "selftext": "", "subreddit": "a"}
for i in range(reddit._FEED_PAGE)]
with patch.object(reddit, "_fetch_subreddit_rss", return_value=full):
out = reddit.fetch_reddit_posts("NVDA", subreddits=("a", "b"))
assert "r/b: <no posts found" not in out
assert f"newest {reddit._FEED_PAGE}" in out