Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ and this project attempts to adhere to [Semantic Versioning](https://semver.org/

## [Unreleased]

### Fixed

- Fixed excessive memory usage in `AsyncWebhookView` and `SyncWebhookView` caused by creating a new `GitHubRouter` instance on each request.

## [0.6.0]

### Added
Expand Down
3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ dev = [
"pytest-cov>=6.0.0",
"pytest-django>=4.9.0",
"pytest-httpx>=0.33.0",
"pytest-memray>=1.7.0",
"pytest-randomly>=3.16.0",
"pytest-xdist>=3.6.1",
"ruff>=0.7.3"
"ruff>=0.7.3",
]
types = [
"django-stubs>=5.1.1",
Expand Down
4 changes: 3 additions & 1 deletion src/django_github_app/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@

GitHubAPIType = TypeVar("GitHubAPIType", AsyncGitHubAPI, SyncGitHubAPI)

_router = GitHubRouter(*GitHubRouter.routers)


class BaseWebhookView(View, ABC, Generic[GitHubAPIType]):
github_api_class: type[GitHubAPIType]
Expand Down Expand Up @@ -59,7 +61,7 @@ def get_response(self, event_log: EventLog) -> JsonResponse:

@property
def router(self) -> GitHubRouter:
return GitHubRouter(*GitHubRouter.routers)
return _router

@abstractmethod
def post(
Expand Down
102 changes: 102 additions & 0 deletions tests/test_routing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
from __future__ import annotations

import pytest
from django.http import HttpRequest
from django.http import JsonResponse

from django_github_app.github import SyncGitHubAPI
from django_github_app.routing import GitHubRouter
from django_github_app.views import BaseWebhookView


@pytest.fixture(autouse=True)
def test_router():
import django_github_app.views
from django_github_app.routing import GitHubRouter

old_routers = GitHubRouter._routers.copy()
GitHubRouter._routers = []

old_router = django_github_app.views._router

test_router = GitHubRouter()
django_github_app.views._router = test_router

yield test_router

GitHubRouter._routers = old_routers
django_github_app.views._router = old_router


class View(BaseWebhookView[SyncGitHubAPI]):
github_api_class = SyncGitHubAPI

def post(self, request: HttpRequest) -> JsonResponse:
return JsonResponse({})


class LegacyView(BaseWebhookView[SyncGitHubAPI]):
github_api_class = SyncGitHubAPI

@property
def router(self) -> GitHubRouter:
# Always create a new router (simulating issue #73)
return GitHubRouter(*GitHubRouter.routers)

def post(self, request: HttpRequest) -> JsonResponse:
return JsonResponse({})


class TestGitHubRouter:
def test_router_single_instance(self):
view1 = View()
view2 = View()

router1 = view1.router
router2 = view2.router

assert router1 is router2
assert view1.router is router1
assert view2.router is router2

def test_no_duplicate_routers(self):
router_ids = set()

for _ in range(1000):
view = View()
router_ids.add(id(view.router))

assert len(router_ids) == 1

def test_duplicate_routers_without_module_level_router(self):
router_ids = set()

for _ in range(5):
view = LegacyView()
router_ids.add(id(view.router))

assert len(router_ids) == 5

@pytest.mark.limit_memory("2.5MB")
def test_router_memory_stress_test(self):
view_count = 50000
views = []

for _ in range(view_count):
view = View()
views.append(view)

assert len(views) == view_count
assert all(view.router is views[0].router for view in views)

@pytest.mark.limit_memory("4MB")
def test_router_memory_stress_test_legacy(self):
view_count = 50000
views = []

for _ in range(view_count):
view = LegacyView()
views.append(view)

assert len(views) == view_count
assert not all(view.router is views[0].router for view in views)
17 changes: 14 additions & 3 deletions tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
from django_github_app.github import AsyncGitHubAPI
from django_github_app.github import SyncGitHubAPI
from django_github_app.models import EventLog
from django_github_app.routing import GitHubRouter
from django_github_app.views import AsyncWebhookView
from django_github_app.views import BaseWebhookView
from django_github_app.views import SyncWebhookView
Expand Down Expand Up @@ -71,9 +70,21 @@ def _make_request(

@pytest.fixture
def test_router():
import django_github_app.views
from django_github_app.routing import GitHubRouter

old_routers = GitHubRouter._routers.copy()
GitHubRouter._routers = []
yield GitHubRouter()
GitHubRouter._routers = []

old_router = django_github_app.views._router

test_router = GitHubRouter()
django_github_app.views._router = test_router

yield test_router

GitHubRouter._routers = old_routers
django_github_app.views._router = old_router


@pytest.fixture
Expand Down
Loading