LCORE-1438: Organize imports in quota handler#1323
Conversation
WalkthroughThis PR reorganizes imports across the quota module files, adding Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/quota/revokable_quota_limiter.py`:
- Around line 12-13: available_quota() races on initial row creation because
INIT_QUOTA_PG and INIT_QUOTA_SQLITE are plain INSERTs; make initialization
idempotent by changing INIT_QUOTA_PG to use a conflict-tolerant INSERT (e.g.
INSERT ... ON CONFLICT DO NOTHING) and INIT_QUOTA_SQLITE to use SQLite's
conflict form (e.g. INSERT OR IGNORE), or alternatively wrap the INSERT in
available_quota() with a duplicate-key handler that swallows the
unique-constraint error and proceeds to return initial_quota; update the SQL
constants INIT_QUOTA_PG and INIT_QUOTA_SQLITE (or add the duplicate-key catch in
available_quota()) so concurrent first requests cannot cause a duplicate-row
failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c93d21d4-0170-4f95-817b-04e60dacb002
📒 Files selected for processing (7)
src/quota/cluster_quota_limiter.pysrc/quota/connect_pg.pysrc/quota/quota_limiter.pysrc/quota/quota_limiter_factory.pysrc/quota/revokable_quota_limiter.pysrc/quota/token_usage_history.pysrc/quota/user_quota_limiter.py
| INIT_QUOTA_PG, | ||
| INIT_QUOTA_SQLITE, |
There was a problem hiding this comment.
Make quota initialization idempotent.
available_quota() follows a read-then-initialize flow. With INIT_QUOTA_PG / INIT_QUOTA_SQLITE defined as plain INSERTs in src/quota/sql.py:60-68, two concurrent first requests for the same subject can both see "missing" and one will fail on the duplicate row. Please make the init SQL conflict-tolerant, or catch duplicate-key violations before returning initial_quota.
Possible fix
-INIT_QUOTA_PG = """
- INSERT INTO quota_limits (id, subject, quota_limit, available, revoked_at)
- VALUES (%s, %s, %s, %s, %s)
- """
+INIT_QUOTA_PG = """
+ INSERT INTO quota_limits (id, subject, quota_limit, available, revoked_at)
+ VALUES (%s, %s, %s, %s, %s)
+ ON CONFLICT DO NOTHING
+ """
-INIT_QUOTA_SQLITE = """
- INSERT INTO quota_limits (id, subject, quota_limit, available, revoked_at)
- VALUES (?, ?, ?, ?, ?)
- """
+INIT_QUOTA_SQLITE = """
+ INSERT OR IGNORE INTO quota_limits (id, subject, quota_limit, available, revoked_at)
+ VALUES (?, ?, ?, ?, ?)
+ """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/quota/revokable_quota_limiter.py` around lines 12 - 13, available_quota()
races on initial row creation because INIT_QUOTA_PG and INIT_QUOTA_SQLITE are
plain INSERTs; make initialization idempotent by changing INIT_QUOTA_PG to use a
conflict-tolerant INSERT (e.g. INSERT ... ON CONFLICT DO NOTHING) and
INIT_QUOTA_SQLITE to use SQLite's conflict form (e.g. INSERT OR IGNORE), or
alternatively wrap the INSERT in available_quota() with a duplicate-key handler
that swallows the unique-constraint error and proceeds to return initial_quota;
update the SQL constants INIT_QUOTA_PG and INIT_QUOTA_SQLITE (or add the
duplicate-key catch in available_quota()) so concurrent first requests cannot
cause a duplicate-row failure.
Description
LCORE-1438: Organize imports in quota handler
Type of change
Tools used to create PR
Related Tickets & Documents
Summary by CodeRabbit