refactor: code-blue source rate limit config management#1595
refactor: code-blue source rate limit config management#1595felixschmetz wants to merge 1 commit intomainfrom
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 16 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="backend/airweave/domains/source_rate_limits/service.py">
<violation number="1" location="backend/airweave/domains/source_rate_limits/service.py:27">
P1: Custom agent: **Explicit Protocol Implementation**
Explicitly inherit `SourceRateLimitServiceProtocol` on `SourceRateLimitService` to satisfy the protocol-implementation requirement.</violation>
</file>
<file name="backend/airweave/domains/source_rate_limits/fakes/repository.py">
<violation number="1" location="backend/airweave/domains/source_rate_limits/fakes/repository.py:15">
P1: Custom agent: **Explicit Protocol Implementation**
Explicitly inherit `SourceRateLimitRepositoryProtocol` on this repository fake to satisfy the protocol implementation rule.</violation>
</file>
<file name="backend/airweave/domains/source_rate_limits/fakes/service.py">
<violation number="1" location="backend/airweave/domains/source_rate_limits/fakes/service.py:16">
P1: Custom agent: **Explicit Protocol Implementation**
Explicitly inherit `SourceRateLimitServiceProtocol` on this service class to comply with the Explicit Protocol Implementation rule.</violation>
</file>
<file name="backend/airweave/domains/source_rate_limits/repository.py">
<violation number="1" location="backend/airweave/domains/source_rate_limits/repository.py:19">
P1: Custom agent: **Explicit Protocol Implementation**
Explicitly inherit `SourceRateLimitRepositoryProtocol` on this repository class to comply with the Explicit Protocol Implementation rule.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| self, db: AsyncSession, *, org_id: UUID, source_short_name: str | ||
| ) -> Optional[SourceRateLimit]: | ||
| """Get rate limit config for an org+source pair.""" | ||
| ... |
Check notice
Code scanning / CodeQL
Statement has no effect Note
| self, db: AsyncSession, *, org_id: UUID | ||
| ) -> list[SourceRateLimit]: | ||
| """Get all configured rate limits for an organization.""" | ||
| ... |
Check notice
Code scanning / CodeQL
Statement has no effect Note
| self, db: AsyncSession, *, obj_in: SourceRateLimitCreate, ctx: ApiContext | ||
| ) -> SourceRateLimit: | ||
| """Create a new rate limit configuration.""" | ||
| ... |
Check notice
Code scanning / CodeQL
Statement has no effect Note
| ctx: ApiContext, | ||
| ) -> SourceRateLimit: | ||
| """Update an existing rate limit configuration.""" | ||
| ... |
Check notice
Code scanning / CodeQL
Statement has no effect Note
|
|
||
| async def remove(self, db: AsyncSession, *, id: UUID, ctx: ApiContext) -> None: | ||
| """Delete a rate limit configuration by ID.""" | ||
| ... |
Check notice
Code scanning / CodeQL
Statement has no effect Note
|
|
||
| async def get_all_sources(self, db: AsyncSession) -> list[Source]: | ||
| """Get all sources (for building the merged rate-limit list).""" | ||
| ... |
Check notice
Code scanning / CodeQL
Statement has no effect Note
| self, db: AsyncSession, *, ctx: ApiContext | ||
| ) -> list[SourceRateLimitResponse]: | ||
| """List all sources merged with their configured rate limits.""" | ||
| ... |
Check notice
Code scanning / CodeQL
Statement has no effect Note
| ctx: ApiContext, | ||
| ) -> SourceRateLimit: | ||
| """Create or update a rate limit configuration for a source.""" | ||
| ... |
Check notice
Code scanning / CodeQL
Statement has no effect Note
| self, db: AsyncSession, *, source_short_name: str, ctx: ApiContext | ||
| ) -> None: | ||
| """Remove rate limit configuration for a source.""" | ||
| ... |
Check notice
Code scanning / CodeQL
Statement has no effect Note
Extract source rate limit configuration CRUD from the endpoint and helpers into a proper domain module under domains/source_rate_limits/ with protocols, repository, service, fakes, and table-driven tests. - Add SourceRateLimitServiceProtocol + SourceRateLimitRepositoryProtocol - Reimplement list/set/delete logic in SourceRateLimitService (not delegation) - Wire into Container + factory, add fake fixture to conftest - Endpoint is now a thin 3-line router via Inject() - Delete core/source_rate_limit_helpers.py (sole consumer was the endpoint) - Move Pipedream proxy constants to domains/source_rate_limits/types.py
felixschmetz
force-pushed
the
refactor/source-rate-limits
branch
from
507ef00 to
2ad44c0
Compare
| cached = await self._redis.get(cache_key) | ||
| if cached: | ||
| return cached if cached != "None" else None | ||
| except Exception: |
Check notice
Code scanning / CodeQL
Empty except Note
|
|
||
| try: | ||
| await self._redis.setex(cache_key, 600, rate_limit_level or "None") | ||
| except Exception: |
Check notice
Code scanning / CodeQL
Empty except Note
| if not rate_limit_obj: | ||
| try: | ||
| await self._redis.setex(cache_key, CONFIG_CACHE_TTL, "{}") | ||
| except Exception: |
Check notice
Code scanning / CodeQL
Empty except Note
| Raises: | ||
| SourceRateLimitExceededException: If rate limit is exceeded. | ||
| """ | ||
| ... |
Check notice
Code scanning / CodeQL
Statement has no effect Note
| Raises: | ||
| SourceRateLimitExceededException: If Pipedream proxy limit exceeded. | ||
| """ | ||
| ... |
Check notice
Code scanning / CodeQL
Statement has no effect Note
1 participant
Extract source rate limit configuration CRUD from the endpoint and helpers into a proper domain module under domains/source_rate_limits/ with protocols, repository, service, fakes, and table-driven tests.
Summary by cubic
Refactored source rate limit config into a domain module and introduced an adapter-based runtime limiter with DI across the stack. Behavior stays the same; Pipedream defaults are centralized and tests cover the config service and Redis limiter.
SourceRateLimiterprotocol and adapters:redis(sliding window via Lua),null, andfake, with unit tests.SourceLifecycleServicetoAirweaveHttpClient, which now uses the injected limiter.SourceRateLimitServiceandSourceRateLimitRepositorywith protocols, fakes, and table-driven tests; preserved feature flag checks, list sorting, and upsert/delete.SourceRateLimitServicevia the container; removedcore/source_rate_limit_helpers.py.domains/source_rate_limits/types.pyand used them in both the service and limiter; removedcore/source_rate_limiter_service.py.Written for commit 2ad44c0. Summary will update on new commits.