Skip to content

Conversation

@sl0thentr0py
Copy link
Member

@sl0thentr0py sl0thentr0py commented Jul 22, 2025

Since we're using current_thread for self explicitly, we need to remove the first argument from *a.

This actually doesn't matter since *a, **kw are both empty but since this is the way the patch is implemented (presumably for forward compat), I don't want to change the signature.

This fixes the following warning in CI since what actually happens is this:

  • Thread.run is being patched at each Thread.start (and not just once as other patches do)
  • So the current thread keeps getting accumulated in *a giving the TypeError for subsequent thread runs except the first
 TypeError: Thread.run() takes 1 positional argument but 5 were given

tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py::test_async_span_origin
  /Users/neel/sentry/sdks/sentry-python/.tox/py3.12-redis-v5/lib/python3.12/site-packages/_pytest/threadexception.py:58: PytestUnhandledThreadExceptionWarning: Exception in thread sentry.monitor

closes #4361

@sl0thentr0py sl0thentr0py requested a review from a team as a code owner July 22, 2025 13:59
@codecov
Copy link

codecov bot commented Jul 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.63%. Comparing base (7339704) to head (f738a8a).
Report is 1 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4610      +/-   ##
==========================================
- Coverage   80.65%   80.63%   -0.03%     
==========================================
  Files         156      156              
  Lines       16500    16500              
  Branches     2806     2806              
==========================================
- Hits        13308    13304       -4     
- Misses       2304     2305       +1     
- Partials      888      891       +3     
Files with missing lines Coverage Δ
sentry_sdk/integrations/threading.py 89.33% <100.00%> (-1.34%) ⬇️

... and 3 files with indirect coverage changes

@sl0thentr0py sl0thentr0py merged commit 1b4f8d3 into master Jul 22, 2025
137 checks passed
@sl0thentr0py sl0thentr0py deleted the neel/fix-threading-warning branch July 22, 2025 14:09
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.

Tests warning: Thread.run() takes 1 positional argument but x were given

3 participants