Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add app name to error messages #34

Merged
merged 1 commit into from
Nov 9, 2024
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: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Catch N+1 queries in your Django project.
## Features

- Detects N+1s from missing prefetches and from use of `.defer()`/`.only()`
- Friendly error messages like `N+1 detected on User.followers at myapp/views.py:25 in get_user`
- Friendly error messages like `N+1 detected on social.User.followers at myapp/views.py:25 in get_user`
- Configurable thresholds
- Allow-list
- Well-tested
Expand Down Expand Up @@ -45,7 +45,6 @@ read on!
> there is significant overhead to detecting N+1s, and my benchmarks show that it
> can make your code between 2.5x - 7x slower.


### Celery

If you use Celery, you can configure this using [signals](https://docs.celeryq.dev/en/stable/userguide/signals.html):
Expand Down Expand Up @@ -211,6 +210,7 @@ in your settings. This will give you the full call stack from each time the quer
## Comparison to nplusone

zeal borrows heavily from [nplusone](https://github.com/jmcarp/nplusone), but has some differences:

- zeal also detects N+1 caused by using `.only()` and `.defer()`
- it lets you configure your own threshold for what constitutes an N+1
- it has slightly more helpful error messages that tell you where the N+1 occurred
Expand Down
2 changes: 1 addition & 1 deletion src/zeal/listeners.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def notify(
context.calls[key].append(get_stack())
count = len(context.calls[key])
if count >= self._threshold and instance_key not in context.ignored:
message = f"N+1 detected on {model.__name__}.{field}"
message = f"N+1 detected on {model._meta.app_label}.{model.__name__}.{field}"
self._alert(model, field, message, context.calls[key])
_nplusone_context.set(context)

Expand Down
22 changes: 12 additions & 10 deletions tests/test_listeners.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def test_can_log_errors(settings, caplog):
assert len(w) == 1
assert issubclass(w[0].category, UserWarning)
assert re.search(
r"N\+1 detected on User\.posts at .*\/test_listeners\.py:23 in test_can_log_errors",
r"N\+1 detected on social\.User\.posts at .*\/test_listeners\.py:23 in test_can_log_errors",
str(w[0].message),
)

Expand All @@ -42,7 +42,7 @@ def test_can_log_all_traces(settings):
assert len(w) == 1
assert issubclass(w[0].category, UserWarning)
expected_lines = [
"N+1 detected on User.posts with calls:",
"N+1 detected on social.User.posts with calls:",
"CALL 1:",
"tests/test_listeners.py:41 in test_can_log_all_traces",
"CALL 2:",
Expand All @@ -58,7 +58,7 @@ def test_errors_include_caller():
PostFactory.create(author=user_2)
with pytest.raises(
NPlusOneError,
match=r"N\+1 detected on User\.posts at .*\/test_listeners\.py:64 in test_errors_include_caller",
match=r"N\+1 detected on social\.User\.posts at .*\/test_listeners\.py:64 in test_errors_include_caller",
):
for user in User.objects.all():
_ = list(user.posts.all())
Expand All @@ -76,7 +76,7 @@ def test_can_exclude_with_allowlist(settings):
_ = list(user.posts.all())

with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on Post.author")
NPlusOneError, match=re.escape("N+1 detected on social.Post.author")
):
for post in Post.objects.all():
_ = post.author
Expand All @@ -94,7 +94,7 @@ def test_can_use_fnmatch_pattern_in_allowlist_model(settings):
_ = list(user.posts.all())

with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on Post.author")
NPlusOneError, match=re.escape("N+1 detected on social.Post.author")
):
for post in Post.objects.all():
_ = post.author
Expand All @@ -112,7 +112,7 @@ def test_can_use_fnmatch_pattern_in_allowlist_field(settings):
_ = list(user.posts.all())

with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on Post.author")
NPlusOneError, match=re.escape("N+1 detected on social.Post.author")
):
for post in Post.objects.all():
_ = post.author
Expand Down Expand Up @@ -189,7 +189,8 @@ def test_can_ignore_specific_models():
_ = list(user.posts.all())

with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on Post.author")
NPlusOneError,
match=re.escape("N+1 detected on social.Post.author"),
):
# this is *not* allowlisted
for post in Post.objects.all():
Expand All @@ -198,14 +199,14 @@ def test_can_ignore_specific_models():
# if we ignore another field, we still raise
with zeal_ignore([{"model": "social.User", "field": "foobar"}]):
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.posts")
NPlusOneError, match=re.escape("N+1 detected on social.User.posts")
):
for user in User.objects.all():
_ = list(user.posts.all())

# outside of the context, we're back to normal
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.posts")
NPlusOneError, match=re.escape("N+1 detected on social.User.posts")
):
for user in User.objects.all():
_ = list(user.posts.all())
Expand All @@ -222,7 +223,8 @@ def test_context_specific_allowlist_merges():
_ = list(user.posts.all())

with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on Post.author")
NPlusOneError,
match=re.escape("N+1 detected on social.Post.author"),
):
# this is *not* allowlisted
for post in Post.objects.all():
Expand Down
32 changes: 16 additions & 16 deletions tests/test_nplusones.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def test_detects_nplusone_in_forward_many_to_one():
PostFactory.create(author=user_1)
PostFactory.create(author=user_2)
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on Post.author")
NPlusOneError, match=re.escape("N+1 detected on social.Post.author")
):
for post in Post.objects.all():
_ = post.author.username
Expand All @@ -32,7 +32,7 @@ def test_detects_nplusone_in_forward_many_to_one_iterator():
PostFactory.create(author=user)

with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on Post.author")
NPlusOneError, match=re.escape("N+1 detected on social.Post.author")
):
for post in Post.objects.all().iterator(chunk_size=2):
_ = post.author.username
Expand Down Expand Up @@ -89,7 +89,7 @@ def test_detects_nplusone_in_reverse_many_to_one():
PostFactory.create(author=user_1)
PostFactory.create(author=user_2)
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.posts")
NPlusOneError, match=re.escape("N+1 detected on social.User.posts")
):
for user in User.objects.all():
_ = list(user.posts.all())
Expand All @@ -103,7 +103,7 @@ def test_detects_nplusone_in_reverse_many_to_one_iterator():
user = UserFactory.create()
PostFactory.create(author=user)
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.posts")
NPlusOneError, match=re.escape("N+1 detected on social.User.posts")
):
for user in User.objects.all().iterator(chunk_size=2):
_ = list(user.posts.all())
Expand All @@ -128,7 +128,7 @@ def test_detects_nplusone_in_forward_one_to_one():
ProfileFactory.create(user=user_1)
ProfileFactory.create(user=user_2)
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on Profile.user")
NPlusOneError, match=re.escape("N+1 detected on social.Profile.user")
):
for profile in Profile.objects.all():
_ = profile.user.username
Expand All @@ -142,7 +142,7 @@ def test_detects_nplusone_in_forward_one_to_one_iterator():
user = UserFactory.create()
ProfileFactory.create(user=user)
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on Profile.user")
NPlusOneError, match=re.escape("N+1 detected on social.Profile.user")
):
for profile in Profile.objects.all().iterator(chunk_size=2):
_ = profile.user.username
Expand Down Expand Up @@ -198,7 +198,7 @@ def test_detects_nplusone_in_reverse_one_to_one():
ProfileFactory.create(user=user_1)
ProfileFactory.create(user=user_2)
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.profile")
NPlusOneError, match=re.escape("N+1 detected on social.User.profile")
):
for user in User.objects.all():
_ = user.profile.display_name
Expand All @@ -212,7 +212,7 @@ def test_detects_nplusone_in_reverse_one_to_one_iterator():
user = UserFactory.create()
ProfileFactory.create(user=user)
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.profile")
NPlusOneError, match=re.escape("N+1 detected on social.User.profile")
):
for user in User.objects.all().iterator(chunk_size=2):
_ = user.profile.display_name
Expand Down Expand Up @@ -267,7 +267,7 @@ def test_detects_nplusone_in_forward_many_to_many():
user_1.following.add(user_2)
user_2.following.add(user_1)
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.following")
NPlusOneError, match=re.escape("N+1 detected on social.User.following")
):
for user in User.objects.all():
_ = list(user.following.all())
Expand All @@ -282,7 +282,7 @@ def test_detects_nplusone_in_forward_many_to_many_iterator():
influencer.followers.set(users) # type: ignore

with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.following")
NPlusOneError, match=re.escape("N+1 detected on social.User.following")
):
for user in User.objects.iterator(chunk_size=2):
_ = list(user.following.all())
Expand Down Expand Up @@ -319,7 +319,7 @@ def test_detects_nplusone_in_reverse_many_to_many():
user_1.following.add(user_2)
user_2.following.add(user_1)
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.followers")
NPlusOneError, match=re.escape("N+1 detected on social.User.followers")
):
for user in User.objects.all():
_ = list(user.followers.all())
Expand All @@ -333,7 +333,7 @@ def test_detects_nplusone_in_reverse_many_to_many_iterator():
users = UserFactory.create_batch(4)
follower.following.set(users) # type: ignore
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.followers")
NPlusOneError, match=re.escape("N+1 detected on social.User.followers")
):
for user in User.objects.all().iterator(chunk_size=2):
_ = list(user.followers.all())
Expand Down Expand Up @@ -370,7 +370,7 @@ def test_detects_nplusone_in_reverse_many_to_many_with_no_related_name():
user_1.blocked.add(user_2)
user_2.blocked.add(user_1)
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.user_set")
NPlusOneError, match=re.escape("N+1 detected on social.User.user_set")
):
for user in User.objects.all():
_ = list(user.user_set.all())
Expand All @@ -384,7 +384,7 @@ def test_detects_nplusone_due_to_deferred_fields():
PostFactory.create(author=user_1)
PostFactory.create(author=user_2)
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.username")
NPlusOneError, match=re.escape("N+1 detected on social.User.username")
):
for post in (
Post.objects.all().select_related("author").only("author__id")
Expand All @@ -402,7 +402,7 @@ def test_detects_nplusone_due_to_deferred_fields_in_iterator():
user = UserFactory.create()
PostFactory.create(author=user)
with pytest.raises(
NPlusOneError, match=re.escape("N+1 detected on User.username")
NPlusOneError, match=re.escape("N+1 detected on social.User.username")
):
for post in (
Post.objects.all()
Expand Down Expand Up @@ -468,7 +468,7 @@ def test_works_in_web_requests(client):
ProfileFactory.create(user=user_1)
ProfileFactory.create(user=user_2)
with pytest.raises(
NPlusOneError, match=re.escape(r"N+1 detected on User.profile")
NPlusOneError, match=re.escape(r"N+1 detected on social.User.profile")
):
response = client.get("/users/")

Expand Down