-
Notifications
You must be signed in to change notification settings - Fork 550
Fix CI, adapt to new redis-py release #4431
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
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
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
|
@@ -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], |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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
There was a problem hiding this 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!
Three new versions of things that need addressing:
event_loop
fixtureexecution_strategy
abstraction to async cluster pipelines as well, so the commands are no longer accessible directly and need to be accessed via thestrategy
_client
attribute on async cluster pipeline tocluster_client