fix(opendata): rate-limit nl_rechtspraak importer, opt-in court scope filter - #2219
fix(opendata): rate-limit nl_rechtspraak importer, opt-in court scope filter#2219teosoph wants to merge 1 commit into
Conversation
…scope filter data.rechtspraak.nl documents a 10 req/sec fair-use cap; a plain worker-count throttle already measured at 11.7 req/s during pilot testing, so add a real sliding-window rate limiter (shared/http_client.py) and enable it for this importer at 8/sec. Also add an optional ECLI-court-code scope filter (PILOT_COURTS, default ALL — no behavior change) and a START_DATE override for cold-start historical backfills, both opt-in via env var. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="services/opendata-importers/importers/nl_rechtspraak.py">
<violation number="1" location="services/opendata-importers/importers/nl_rechtspraak.py:35">
P2: Court lists containing conventional spaces after commas silently exclude those courts. Normalize each `PILOT_COURTS` item before comparing it to uppercase ECLI court codes.</violation>
</file>
<file name="services/opendata-importers/shared/http_client.py">
<violation number="1" location="services/opendata-importers/shared/http_client.py:38">
P3: `RateLimiter.acquire()` crashes with `IndexError` if `max_per_sec` is 0 or negative. When the rate limiter is initialized with a non-positive limit, the first call enters `acquire()`, finds the deque empty, skips the eviction while-loop, evaluates `len(self._times) < self.max_per_sec` as False (0 < 0 or 0 < -N), and then reaches `self._times[0]` on an empty deque.
This won't trigger through the current `base.py` code path since `if self.RATE_LIMIT_PER_SEC:` uses truthiness (0.0 is falsy). But the `RateLimiter` class is public and could be used directly or extended with a computed value. Adding a guard in `__init__` would make it robust.
**Suggestion**: In `__init__`, validate and clamp: `if max_per_sec <= 0: raise ValueError("max_per_sec must be positive")`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| _PILOT_COURTS = None if _PILOT_COURTS_ENV.strip().upper() == "ALL" else set( | ||
| _PILOT_COURTS_ENV.split(",")) |
There was a problem hiding this comment.
P2: Court lists containing conventional spaces after commas silently exclude those courts. Normalize each PILOT_COURTS item before comparing it to uppercase ECLI court codes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At services/opendata-importers/importers/nl_rechtspraak.py, line 35:
<comment>Court lists containing conventional spaces after commas silently exclude those courts. Normalize each `PILOT_COURTS` item before comparing it to uppercase ECLI court codes.</comment>
<file context>
@@ -26,6 +26,21 @@
+# unchanged; this is strictly opt-in for a narrower pilot/test run.
+# ECLI shape is ECLI:NL:<court-code>:<year>:<ordinal>.
+_PILOT_COURTS_ENV = os.environ.get("PILOT_COURTS", "ALL")
+_PILOT_COURTS = None if _PILOT_COURTS_ENV.strip().upper() == "ALL" else set(
+ _PILOT_COURTS_ENV.split(","))
+_ECLI_COURT_RE = re.compile(r"^ECLI:NL:([A-Z]+):")
</file context>
| _PILOT_COURTS = None if _PILOT_COURTS_ENV.strip().upper() == "ALL" else set( | |
| _PILOT_COURTS_ENV.split(",")) | |
| _PILOT_COURTS = None if _PILOT_COURTS_ENV.strip().upper() == "ALL" else { | |
| court.strip().upper() for court in _PILOT_COURTS_ENV.split(",") if court.strip() | |
| } |
| if len(self._times) < self.max_per_sec: | ||
| self._times.append(now) | ||
| return | ||
| sleep_for = 1.0 - (now - self._times[0]) |
There was a problem hiding this comment.
P3: RateLimiter.acquire() crashes with IndexError if max_per_sec is 0 or negative. When the rate limiter is initialized with a non-positive limit, the first call enters acquire(), finds the deque empty, skips the eviction while-loop, evaluates len(self._times) < self.max_per_sec as False (0 < 0 or 0 < -N), and then reaches self._times[0] on an empty deque.
This won't trigger through the current base.py code path since if self.RATE_LIMIT_PER_SEC: uses truthiness (0.0 is falsy). But the RateLimiter class is public and could be used directly or extended with a computed value. Adding a guard in __init__ would make it robust.
Suggestion: In __init__, validate and clamp: if max_per_sec <= 0: raise ValueError("max_per_sec must be positive").
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At services/opendata-importers/shared/http_client.py, line 38:
<comment>`RateLimiter.acquire()` crashes with `IndexError` if `max_per_sec` is 0 or negative. When the rate limiter is initialized with a non-positive limit, the first call enters `acquire()`, finds the deque empty, skips the eviction while-loop, evaluates `len(self._times) < self.max_per_sec` as False (0 < 0 or 0 < -N), and then reaches `self._times[0]` on an empty deque.
This won't trigger through the current `base.py` code path since `if self.RATE_LIMIT_PER_SEC:` uses truthiness (0.0 is falsy). But the `RateLimiter` class is public and could be used directly or extended with a computed value. Adding a guard in `__init__` would make it robust.
**Suggestion**: In `__init__`, validate and clamp: `if max_per_sec <= 0: raise ValueError("max_per_sec must be positive")`.</comment>
<file context>
@@ -10,11 +12,38 @@
+ if len(self._times) < self.max_per_sec:
+ self._times.append(now)
+ return
+ sleep_for = 1.0 - (now - self._times[0])
+ await asyncio.sleep(max(sleep_for, 0.01))
+
</file context>
Summary
shared/http_client.py) shared across all workers/IPs in aMultiIPSessionPool, wired in via an opt-inRATE_LIMIT_PER_SEConBaseImporter.nl_rechtspraakat 8 req/sec —data.rechtspraak.nldocuments a 10 req/sec fair-use cap, and a plain 4-worker concurrency setting was measured hitting 11.7 req/s during pilot testing (worker count alone doesn't guarantee staying under a documented limit when per-request latency is low).PILOT_COURTSenv var, defaultALL— no behavior change for existing deployments) for narrowing a run to specific courts (e.g.HR,GHAMS,GHARL,GHDHA,GHSHEfor Hoge Raad + Gerechtshoven only).START_DATEoverride forimport_dataset()— useful for a cold-start historical backfill where the existing "resume fromMAX(decision_date)on target" logic would otherwise start from the year-2000 fallback sentinel.All three additions are opt-in via env var / class attribute with defaults that preserve current behavior — no change to
docker-compose.opendata.ymlor any deployed target configuration.Test plan
python3 -m py_compileon all three changed filesPILOT_COURTSset to a 5-court subset and toALL, confirmed the filter includes/excludes correctlydata.rechtspraak.nlopendata-importersDocker image / compose stack (only tested via directpython3 -m importers.nl_rechtspraakexecution)🤖 Generated with Claude Code
Summary by cubic
Adds a shared sliding-window rate limiter and enables it at 8 req/sec for the
nl_rechtspraakimporter to stay under the 10 req/sec fair-use cap. Also adds opt-in court scoping and a start-date override for targeted pilots and backfills; defaults keep current behavior.shared/http_client.pythat caps requests per rolling second across all workers/IPs inMultiIPSessionPool.BaseImporternow acceptsRATE_LIMIT_PER_SEC;nl_rechtspraaksets it to 8.0.PILOT_COURTS(comma-separated ECLI court codes;ALLor unset disables filtering).START_DATEto start historical runs from a specific day instead ofMAX(decision_date)fallback.Written for commit 0ce455b. Summary will update on new commits.