From 1abaaa537ccec7f76193d1bfd6ffb643014d507d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tao=20Bojl=C3=A9n?= <66130243+taobojlen@users.noreply.github.com> Date: Sat, 9 Nov 2024 10:42:48 +0000 Subject: [PATCH] feat: add app name to error messages --- README.md | 4 ++-- src/zeal/listeners.py | 2 +- tests/test_listeners.py | 22 ++++++++++++---------- tests/test_nplusones.py | 32 ++++++++++++++++---------------- 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 3e932a6..6d8d154 100644 --- a/README.md +++ b/README.md @@ -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 @@ -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): @@ -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 diff --git a/src/zeal/listeners.py b/src/zeal/listeners.py index ba2683d..6c67da7 100644 --- a/src/zeal/listeners.py +++ b/src/zeal/listeners.py @@ -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) diff --git a/tests/test_listeners.py b/tests/test_listeners.py index 75e4c3a..819afe4 100644 --- a/tests/test_listeners.py +++ b/tests/test_listeners.py @@ -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), ) @@ -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:", @@ -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()) @@ -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 @@ -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 @@ -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 @@ -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(): @@ -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()) @@ -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(): diff --git a/tests/test_nplusones.py b/tests/test_nplusones.py index ee63932..a1b8377 100644 --- a/tests/test_nplusones.py +++ b/tests/test_nplusones.py @@ -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 @@ -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 @@ -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()) @@ -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()) @@ -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 @@ -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 @@ -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 @@ -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 @@ -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()) @@ -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()) @@ -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()) @@ -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()) @@ -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()) @@ -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") @@ -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() @@ -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/")