Skip to content

refactor(api): source API_KEY_RATE_LIMIT from settings, drop service token throttle#9161

Merged
sriramveeraghanta merged 1 commit into
previewfrom
refactor/api-key-rate-limit-from-settings
May 28, 2026
Merged

refactor(api): source API_KEY_RATE_LIMIT from settings, drop service token throttle#9161
sriramveeraghanta merged 1 commit into
previewfrom
refactor/api-key-rate-limit-from-settings

Conversation

@sriramveeraghanta
Copy link
Copy Markdown
Member

@sriramveeraghanta sriramveeraghanta commented May 28, 2026

Description

Two small cleanups in the API throttling layer:

  • API_KEY_RATE_LIMIT now flows through Django settings. Added API_KEY_RATE_LIMIT to plane/settings/common.py (defaults to 60/minute, overridable via env). ApiKeyRateThrottle reads settings.API_KEY_RATE_LIMIT via django.conf.settings instead of calling os.environ.get directly. Behavior is unchanged — the same env var and default still apply, but the read path is now consistent with the rest of the project.
  • Removed ServiceTokenRateThrottle. Service tokens are no longer used, so the dedicated 300/minute throttle class and the conditional branch in BaseAPIView.get_throttles that looked up APIToken(is_service=True) per request are gone. All API key requests now go through ApiKeyRateThrottle. This also removes an extra DB query on every API request.

The existing env wiring in deployments/aio/community/* and deployments/cli/community/* already exports API_KEY_RATE_LIMIT — no deployment changes needed.

Type of Change

  • Code refactoring
  • Improvement (removes per-request DB lookup for service-token throttle)

Screenshots and Media (if applicable)

Test Scenarios

  • Hit any /api/v1/... endpoint with X-Api-Key set to a regular API token; verify the X-RateLimit-Remaining / X-RateLimit-Reset response headers appear and counters tick down with each request.
  • Send more than 60 requests/minute with the same API key and verify the 61st gets HTTP 429.
  • Override API_KEY_RATE_LIMIT=10/minute in the API container env and confirm the throttle picks up the new limit after restart.
  • Confirm requests using what used to be a service token (is_service=True) are now subject to the same 60/minute (or env-configured) limit as regular API keys — no separate 300/minute bucket.

References

  • Removes ServiceTokenRateThrottle and is_service-based throttle dispatch in apps/api/plane/api/views/base.py.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Simplified API rate limiting to use centralized configuration settings
    • API key rate limit now configured via settings instead of environment variables (default: 60/minute)
    • Removed service token-based rate limiting

Review Change Stack

…token throttle

- Define API_KEY_RATE_LIMIT in plane/settings/common.py and read it via
  django.conf.settings in ApiKeyRateThrottle instead of os.environ.
- Remove ServiceTokenRateThrottle and the service-token branch in
  BaseAPIView.get_throttles; all API key requests now go through
  ApiKeyRateThrottle.
Copilot AI review requested due to automatic review settings May 28, 2026 18:36
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b98cff2c-1e1e-426a-a10d-49555e848d2e

📥 Commits

Reviewing files that changed from the base of the PR and between f14451a and 1b0acfc.

📒 Files selected for processing (3)
  • apps/api/plane/api/rate_limit.py
  • apps/api/plane/api/views/base.py
  • apps/api/plane/settings/common.py

📝 Walkthrough

Walkthrough

This pull request consolidates API rate limiting configuration by centralizing the API_KEY_RATE_LIMIT setting into Django configuration. Service token–based throttling is removed, and BaseAPIView is simplified to use only ApiKeyRateThrottle unconditionally. The rate limit value is now sourced from a single settings location rather than read directly from environment variables in multiple places.

Changes

API Rate Limiting Consolidation

Layer / File(s) Summary
Settings configuration
apps/api/plane/settings/common.py
New API_KEY_RATE_LIMIT setting is introduced, sourced from the environment variable with a "60/minute" default.
Throttle class refactoring
apps/api/plane/api/rate_limit.py
ApiKeyRateThrottle.rate is updated to read from settings.API_KEY_RATE_LIMIT instead of os.environ, and ServiceTokenRateThrottle class is removed.
View throttling simplification
apps/api/plane/api/views/base.py
Imports are simplified to remove ServiceTokenRateThrottle and APIToken dependencies; BaseAPIView.get_throttles() now unconditionally returns ApiKeyRateThrottle() without header inspection.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A throttle grows clear from the crowded chaff,
One setting to rule them, one single path.
Service tokens fade like the morning dew,
Simpler, cleaner—a fresh, focused view! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main refactoring work: sourcing API_KEY_RATE_LIMIT from Django settings and removing the deprecated service token throttle mechanism.
Description check ✅ Passed The description follows the template with complete sections covering what changed, why, test scenarios, and references. All required template sections are addressed with substantive content.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/api-key-rate-limit-from-settings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the API throttling layer by sourcing the API key rate limit from Django settings instead of reading the env var directly, and removes the now-unused ServiceTokenRateThrottle class along with its per-request DB lookup in BaseAPIView.get_throttles.

Changes:

  • Add API_KEY_RATE_LIMIT setting (default 60/minute, env-overridable) in plane/settings/common.py.
  • ApiKeyRateThrottle.rate now reads from settings.API_KEY_RATE_LIMIT instead of os.environ.get; ServiceTokenRateThrottle class deleted.
  • BaseAPIView.get_throttles simplified to always return [ApiKeyRateThrottle()], removing the per-request APIToken(is_service=True) query.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
apps/api/plane/settings/common.py Adds API_KEY_RATE_LIMIT setting sourced from env with 60/minute default.
apps/api/plane/api/rate_limit.py Switches throttle rate to settings.API_KEY_RATE_LIMIT and removes ServiceTokenRateThrottle.
apps/api/plane/api/views/base.py Drops APIToken/ServiceTokenRateThrottle imports and the service-token branch in get_throttles.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

@sriramveeraghanta sriramveeraghanta merged commit 248f5d6 into preview May 28, 2026
17 of 18 checks passed
@sriramveeraghanta sriramveeraghanta deleted the refactor/api-key-rate-limit-from-settings branch May 28, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants