Open
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new Django management command that runs interactive, local-only integration tests against an API gateway at http://localhost:8787, creating and tearing down DB fixtures and asserting authentication, authorization, and aggregation visibility across several endpoint scenarios. Changes
Sequence DiagramsequenceDiagram
participant User
participant Command as "Management Command\n(test_api_gateway.py)"
participant DB as Database
participant Gateway as "Local API Gateway\nhttp://localhost:8787"
participant Backend as "Backend Service"
User->>Command: run command
Command->>Command: assert settings.DEBUG == True
Command->>User: prompt (ENTER to continue)
User->>Command: confirm
loop per test case
Command->>DB: create fixtures (users, api keys, projects, posts, predictions)
DB-->>Command: fixtures ready
Command->>Gateway: HTTP GET/POST to endpoints (/api/posts/, /api/leaderboards/global/) with/without auth
Gateway->>Backend: route request
Backend-->>Gateway: response
Gateway-->>Command: HTTP response
Command->>Command: validate status & response body
Command->>Command: check aggregation visibility fields
Command->>DB: teardown fixtures
DB-->>Command: teardown result (warn on failure)
end
Command->>Command: aggregate results
Command->>User: print PASS/FAIL per test
alt any failures
Command->>User: exit non-zero
else
Command->>User: exit zero
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
misc/management/commands/test_api_gateway.py (1)
163-355: Extract common aggregation post creation logic to reduce duplication.The functions
setup_aggregations_post,setup_aggregations_post_in_bot_testing_project,setup_aggregations_post_in_benchmarking_project, andsetup_aggregations_post_in_data_access_projectshare ~90% identical code. Only the project source differs.♻️ Proposed refactor to extract helper
def _create_aggregations_post(author: User, project: Project) -> Question: """Creates a binary post with a recency_weighted AggregateForecast in the given project.""" now = timezone.now() question = Question.objects.create( type=Question.QuestionType.BINARY, title=_AGGREGATIONS_POST_TITLE, open_time=now - timedelta(days=1), scheduled_close_time=now + timedelta(days=30), scheduled_resolve_time=now + timedelta(days=31), ) post = Post.objects.create( title=_AGGREGATIONS_POST_TITLE, author=author, question=question, curation_status=Post.CurationStatus.APPROVED, published_at=now - timedelta(days=1), default_project=project, open_time=now - timedelta(days=1), scheduled_close_time=now + timedelta(days=30), scheduled_resolve_time=now + timedelta(days=31), hotness=1e10, ) Forecast.objects.create( question=question, post=post, author=author, start_time=now - timedelta(hours=1), probability_yes=0.7, ) AggregateForecast.objects.create( question=question, method=AggregationMethod.RECENCY_WEIGHTED, start_time=now - timedelta(hours=1), end_time=None, forecast_values=[0.3, 0.7], forecaster_count=1, ) return question def setup_aggregations_post(state: dict) -> dict: site_main = Project.objects.filter(type=Project.ProjectTypes.SITE_MAIN).first() if site_main is None: raise RuntimeError("SITE_MAIN project not found") return {"question": _create_aggregations_post(state["user"], site_main)} def setup_aggregations_post_in_bot_testing_project(state: dict) -> dict: return {"question": _create_aggregations_post(state["user"], state["bot_testing_project"])} def setup_aggregations_post_in_benchmarking_project(state: dict) -> dict: return {"question": _create_aggregations_post(state["user"], state["benchmarking_project"])} def setup_aggregations_post_in_data_access_project(state: dict) -> dict: return {"question": _create_aggregations_post(state["user"], state["data_access_project"])}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@misc/management/commands/test_api_gateway.py` around lines 163 - 355, The four setup functions (setup_aggregations_post, setup_aggregations_post_in_bot_testing_project, setup_aggregations_post_in_benchmarking_project, setup_aggregations_post_in_data_access_project) duplicate nearly identical creation logic; extract that logic into a single helper (e.g. _create_aggregations_post(author, project)) that creates the Question, Post, Forecast, and AggregateForecast and returns the Question, then have each setup_* call the helper and return {"question": question}; ensure setup_aggregations_post still resolves the SITE_MAIN project (using Project.objects.filter(...).first()) and raises a clear RuntimeError if not found.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@misc/management/commands/test_api_gateway.py`:
- Around line 370-371: Add a REQUEST_TIMEOUT constant (e.g., REQUEST_TIMEOUT =
30) alongside GATEWAY_BASE_URL and update every HTTP call in this file that uses
requests.get (including the call that builds response =
requests.get(f"{GATEWAY_BASE_URL}/api/posts/") and the other occurrences around
lines noted) to pass timeout=REQUEST_TIMEOUT so none of the requests can hang
indefinitely; ensure every requests.get(...) now includes the timeout kwarg
(timeout=REQUEST_TIMEOUT).
- Around line 114-121: The helper setup_global_leaderboard currently assumes
Project.objects.filter(...).first() returns a Project; add a defensive check
after retrieving site_main in setup_global_leaderboard and in
setup_aggregations_post to handle None: if site_main is None raise a clear
exception (e.g., ValueError or RuntimeError) or log and abort the test setup
with a descriptive message like "SITE_MAIN project not found" so leaderboard
creation (Leaderboard.objects.create in setup_global_leaderboard) and subsequent
code do not fail with confusing attribute errors; reference the site_main
variable and the functions setup_global_leaderboard and setup_aggregations_post
when making the change.
- Around line 20-22: The Unix-only imports (termios, tty) cause ImportError on
Windows; guard them and provide a fallback used by _read_single_keypress: wrap
the termios/tty imports in a try/except and set a boolean like _HAS_TERMIOS,
then update _read_single_keypress to use the termios/tty raw-read logic only
when _HAS_TERMIOS is True and otherwise fall back to a cross-platform approach
(e.g. call input() and return its first character or "\n"); alternatively
mention using a cross-platform library such as click or readchar if you prefer
that route.
---
Nitpick comments:
In `@misc/management/commands/test_api_gateway.py`:
- Around line 163-355: The four setup functions (setup_aggregations_post,
setup_aggregations_post_in_bot_testing_project,
setup_aggregations_post_in_benchmarking_project,
setup_aggregations_post_in_data_access_project) duplicate nearly identical
creation logic; extract that logic into a single helper (e.g.
_create_aggregations_post(author, project)) that creates the Question, Post,
Forecast, and AggregateForecast and returns the Question, then have each setup_*
call the helper and return {"question": question}; ensure
setup_aggregations_post still resolves the SITE_MAIN project (using
Project.objects.filter(...).first()) and raises a clear RuntimeError if not
found.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2f82ec56-6423-4197-8c83-d53f33c3d63f
📒 Files selected for processing (1)
misc/management/commands/test_api_gateway.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
misc/management/commands/test_api_gateway.py (2)
163-205: Collapse the duplicated aggregation fixture setup into one helper.These four setup helpers all build the same question/post/forecast/aggregate graph and only vary the target project. A shared builder would reduce drift and centralize future schema/default changes in one place.
Also applies to: 208-249, 252-293, 315-355
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@misc/management/commands/test_api_gateway.py` around lines 163 - 205, Multiple test helpers duplicate the same question/post/forecast/aggregate creation; refactor by extracting a single reusable builder (e.g., create_aggregation_fixture) that encapsulates the shared creation logic used in setup_aggregations_post and the other three helpers, and accept a parameter for the target project/default_project to vary; update setup_aggregations_post to call this new create_aggregation_fixture (passing site_main) and replace the other duplicate helpers to call it with their respective project values so all creation logic (Question.objects.create, Post.objects.create, Forecast.objects.create, AggregateForecast.objects.create, use of timezone.now(), and hotness/default fields) lives in one place.
62-104: Make fixture identifiers unique per invocation.The usernames/emails, leaderboard name, and aggregation post title are reused on every run. After an aborted or partially cleaned-up run, reruns can collide with or accidentally validate stale data. Seeding a
run_idonce and appending it to generated identifiers would make these manual checks repeatable.Also applies to: 111-111, 160-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@misc/management/commands/test_api_gateway.py` around lines 62 - 104, Generate a single run_id once per test invocation (e.g., UUID or timestamp) and append it to all created identifiers; update the user fixture factories (setup_restricted_user, setup_staff_user, setup_benchmarking_tier_user, setup_unrestricted_tier_user) to derive username and email from f"{base}-{run_id}" so they are unique on each run, and likewise append the same run_id to the leaderboard name and aggregation post title fixtures referenced elsewhere so those resources also use run-specific names; store run_id in the shared state dict (or a module-level variable initialized in test setup) and use it when constructing all identifiers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@misc/management/commands/test_api_gateway.py`:
- Around line 579-590: The loop currently prints "PASS" immediately after
test_fn(state) and only logs teardown exceptions as WARN, so teardown failures
don't fail the run; change the sequence in the try/except/finally around test_fn
to: run test_fn(state) but do NOT print PASS yet, run all teardown_fns(state)
inside finally collecting any teardown exceptions (append (name, teardown_exc)
to failures for each), and only print "PASS {name}" if neither the test raised
nor any teardown raised; keep the existing except block to append (name, exc)
for test failures (and skip printing PASS), and ensure teardown_fns, failures,
name, test_fn and state are used to locate and implement these changes.
- Around line 417-420: The test currently asserts and returns data from
results[0], which may not be the fixture for this scenario; instead pass a
run-unique identifier through state when creating the fixture (e.g. a unique
title/id stored in state), then after response = client.get(...) parse results =
response.json().get("results", []), locate the matching result by that unique
identifier (e.g. next(r for r in results if r["title"] == state["unique_title"]
or r["id"] == state["unique_id"])), use that matched_result in the assert_equal
call (replace "results[0].title" and results[0]["title"] with the matched result
references) and return
matched_result["question"]["aggregations"]["recency_weighted"]["latest"]; this
ensures the assertions reference the scenario-specific fixture rather than
results[0].
- Around line 368-562: This file defines top-level pytest-discoverable functions
named test_* (e.g., test_unauthenticated_get_posts,
test_authenticated_get_posts, test_restricted_user_blocked_endpoint,
test_staff_can_access_blocked_endpoint, test_restricted_aggregations_hidden,
test_unrestricted_aggregations_visible, test_staff_aggregations_visible,
test_project_data_access_user_sees_cp,
test_restricted_cp_hidden_in_benchmarking_project,
test_benchmarking_cp_visible_in_benchmarking_project,
test_restricted_aggregations_visible_in_bot_testing_project); rename each of
these defs to a non-pytest name (e.g., scenario_... or integration_test_...) and
update the TESTS registry so the first element of each tuple references the new
function names (keep docstrings and other logic unchanged) to prevent pytest
from collecting them while preserving the test registry behavior.
---
Nitpick comments:
In `@misc/management/commands/test_api_gateway.py`:
- Around line 163-205: Multiple test helpers duplicate the same
question/post/forecast/aggregate creation; refactor by extracting a single
reusable builder (e.g., create_aggregation_fixture) that encapsulates the shared
creation logic used in setup_aggregations_post and the other three helpers, and
accept a parameter for the target project/default_project to vary; update
setup_aggregations_post to call this new create_aggregation_fixture (passing
site_main) and replace the other duplicate helpers to call it with their
respective project values so all creation logic (Question.objects.create,
Post.objects.create, Forecast.objects.create, AggregateForecast.objects.create,
use of timezone.now(), and hotness/default fields) lives in one place.
- Around line 62-104: Generate a single run_id once per test invocation (e.g.,
UUID or timestamp) and append it to all created identifiers; update the user
fixture factories (setup_restricted_user, setup_staff_user,
setup_benchmarking_tier_user, setup_unrestricted_tier_user) to derive username
and email from f"{base}-{run_id}" so they are unique on each run, and likewise
append the same run_id to the leaderboard name and aggregation post title
fixtures referenced elsewhere so those resources also use run-specific names;
store run_id in the shared state dict (or a module-level variable initialized in
test setup) and use it when constructing all identifiers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 05bfd586-baa9-40c4-a40f-cf9b04e1262f
📒 Files selected for processing (1)
misc/management/commands/test_api_gateway.py
Contributor
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
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.
Adds python manage.py test_api_gateway — a manually-run black-box test suite for the local
Cloudflare API gateway (port 8787). Tests cover authentication, blocked endpoints, and CP
visibility across all permission tiers (restricted, benchmarking, unrestricted, staff, and
per-project UserDataAccess). Setup/teardown are composable lists of functions for easy reuse.
Includes a DEBUG=True check and confirmation prompt to prevent accidental production runs.
Summary by CodeRabbit
Note: Internal tooling only; no direct impact on user-facing functionality.