feat: improved global 429 management with nginx and improved SQLite usage with mmap#363
Merged
feat: improved global 429 management with nginx and improved SQLite usage with mmap#363
Conversation
Contributor
Reviewer's GuideImplements improved global 429 rate limit handling by wiring Nginx/Valkey-based checks into configuration and clarifying the Python client’s defensive rate-limit check, and adds optional SQLite performance tuning via memory-mapped I/O controlled by configuration. Sequence diagram for improved global 429 rate limit handlingsequenceDiagram
actor Client
participant Nginx as Nginx_OpenResty
participant Valkey as Valkey
participant App as Python_App
participant BClient as BlizzardClient
participant Cache as CacheManager
Client->>Nginx: HTTP request
Nginx->>Valkey: Check global rate limit state
alt Rate limit active in Valkey
Nginx-->>Client: 429 Too Many Requests
else Not rate limited (or cache miss)
Nginx->>App: Forward request
App->>BClient: Execute API-related operation
BClient->>Cache: is_being_rate_limited()
Cache-->>BClient: Rate limit state
alt Rate limited at application level
BClient-->>App: Raise 429 with Retry-After
App-->>Client: 429 Too Many Requests
else Not rate limited
BClient-->>App: Proceed with Blizzard API call
App-->>Client: Success response
end
end
Class diagram for Settings, SQLite storage, and Blizzard client changesclassDiagram
class Settings {
+str storage_path
+int sqlite_mmap_size
}
class SQLiteStorage {
+_get_connection() aiosqlite_Connection
}
class BlizzardClient {
+get(url, params, headers, timeout) httpx_Response
+aclose() None
-_check_rate_limit() None
}
class CacheManager {
+is_being_rate_limited() bool
}
Settings <.. SQLiteStorage : config
SQLiteStorage o-- Settings : uses
BlizzardClient o-- CacheManager : uses
class aiosqlite_Connection {
+execute(sql)
}
SQLiteStorage --> aiosqlite_Connection : manages
class RateLimitConfig {
+str BLIZZARD_RATE_LIMIT_KEY
+int BLIZZARD_RATE_LIMIT_RETRY_AFTER
+str RETRY_AFTER_HEADER
}
RateLimitConfig <.. BlizzardClient
RateLimitConfig <.. CacheManager
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 security issue, and left some high level feedback:
Security issues:
- Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option. (link)
General comments:
- For the SQLite
mmap_sizePRAGMA, consider validating or normalizingsettings.sqlite_mmap_size(e.g., enforcing non-negative values and alignment to page size) before interpolating it into the SQL to avoid unexpected SQLite behavior or errors. - Now that rate limiting and
Retry-Afterbehavior is partially configured via Nginx env vars and partially in the Python client, it may be worth centralizing the configuration (or at least sharing constants) so that changes in retry delay or header naming can’t drift between layers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- For the SQLite `mmap_size` PRAGMA, consider validating or normalizing `settings.sqlite_mmap_size` (e.g., enforcing non-negative values and alignment to page size) before interpolating it into the SQL to avoid unexpected SQLite behavior or errors.
- Now that rate limiting and `Retry-After` behavior is partially configured via Nginx env vars and partially in the Python client, it may be worth centralizing the configuration (or at least sharing constants) so that changes in retry delay or header naming can’t drift between layers.
## Individual Comments
### Comment 1
<location> `app/adapters/storage/sqlite_storage.py:100` </location>
<code_context>
await db.execute(f"PRAGMA mmap_size={settings.sqlite_mmap_size}")
</code_context>
<issue_to_address>
**security (python.sqlalchemy.security.sqlalchemy-execute-raw-query):** Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # This can significantly improve read performance by mapping database | ||
| # pages directly into memory | ||
| if settings.sqlite_mmap_size > 0: | ||
| await db.execute(f"PRAGMA mmap_size={settings.sqlite_mmap_size}") |
Contributor
There was a problem hiding this comment.
security (python.sqlalchemy.security.sqlalchemy-execute-raw-query): Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option.
Source: opengrep
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Improve rate limiting handling via Nginx integration and add configurable SQLite mmap-based performance tuning.
New Features:
Enhancements:
Build: