Skip to content

Fix CI, adapt to new redis-py release #4431

New issue

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

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

Already on GitHub? # to your account

Merged
merged 11 commits into from
Jun 2, 2025
Merged

Conversation

sentrivana
Copy link
Contributor

@sentrivana sentrivana commented Jun 2, 2025

Three new versions of things that need addressing:

  • pytest-asyncio 1.0 removes the deprecated event_loop fixture
  • mypy 1.16.0 finds some new issues
  • redis 6.2.0:
    • adds the execution_strategy abstraction to async cluster pipelines as well, so the commands are no longer accessible directly and need to be accessed via the strategy
    • renames the _client attribute on async cluster pipeline to cluster_client

Copy link

codecov bot commented Jun 2, 2025

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 80.67%. Comparing base (385c668) to head (3d45ded).
Report is 1 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/integrations/django/asgi.py 50.00% 1 Missing ⚠️
sentry_sdk/integrations/tornado.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4431      +/-   ##
==========================================
+ Coverage   80.61%   80.67%   +0.05%     
==========================================
  Files         142      142              
  Lines       15973    15982       +9     
  Branches     2727     2729       +2     
==========================================
+ Hits        12877    12893      +16     
+ Misses       2240     2233       -7     
  Partials      856      856              
Files with missing lines Coverage Δ
sentry_sdk/integrations/redis/_async_common.py 83.92% <100.00%> (+1.92%) ⬆️
sentry_sdk/integrations/redis/redis_cluster.py 82.05% <100.00%> (+1.49%) ⬆️
sentry_sdk/integrations/django/asgi.py 78.07% <50.00%> (ø)
sentry_sdk/integrations/tornado.py 80.53% <0.00%> (ø)

... and 4 files with indirect coverage changes

@sentrivana sentrivana changed the title Test CI Fix CI, adapt to new redis-py release Jun 2, 2025
@@ -188,7 +188,7 @@ class SDKInfo(TypedDict):
"monitor_slug": Optional[str],
"platform": Literal["python"],
"profile": object, # Should be sentry_sdk.profiler.Profile, but we can't import that here due to circular imports
"release": str,
"release": Optional[str],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mypy was complaining because of environment and release potentially being None here

@@ -41,13 +41,21 @@ async def _sentry_execute(self, *args, **kwargs):
origin=SPAN_ORIGIN,
) as span:
with capture_internal_exceptions():
try:
command_seq = self._execution_strategy._command_queue
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redis-py PR that introduced this

@@ -36,11 +36,19 @@ def _set_async_cluster_db_data(span, async_redis_cluster_instance):
def _set_async_cluster_pipeline_db_data(span, async_redis_cluster_pipeline_instance):
# type: (Span, AsyncClusterPipeline[Any]) -> None
with capture_internal_exceptions():
client = getattr(async_redis_cluster_pipeline_instance, "cluster_client", None)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redis-py PR that introduced this

@sentrivana sentrivana marked this pull request as ready for review June 2, 2025 07:57
@sentrivana sentrivana requested a review from a team as a code owner June 2, 2025 07:57
Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lookin' good. thanks for fixing!

@sentrivana sentrivana merged commit aad481d into master Jun 2, 2025
137 checks passed
@sentrivana sentrivana deleted the ivana/fix-ci-aiohttp branch June 2, 2025 08:11
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants