Fix the redis lock during message merging and make more performant #25
Conversation
Codecov ReportAttention: Patch coverage is
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
dkoslicki
left a comment
dkoslicki
left a comment
There was a problem hiding this comment.
Claude is complaining about a possible race condition between subscribing and acquiring the lock. I don't know if you think this matters or not
shepherd_utils/broker.py
Outdated
| pubsub = client.pubsub() | ||
| await pubsub.subscribe(response_id) | ||
| for i in range(12): | ||
| aquired = await client.set(response_id, consumer_id, ex=45, nx=True) |
shepherd_utils/broker.py
Outdated
| await pubsub.subscribe(response_id) | ||
| for i in range(12): | ||
| aquired = await client.set(response_id, consumer_id, ex=45, nx=True) | ||
| if aquired: |
shepherd_utils/broker.py
Outdated
| await pubsub.unsubscribe(response_id) | ||
| await pubsub.aclose() | ||
| await client.aclose() | ||
| return got_lock |
There was a problem hiding this comment.
In the next line, and at other exceptions, do we need to cleanup the pubsub and unsubscribe, or does that not much matter? I do very little async coding, so might not even be a valid question
There was a problem hiding this comment.
I moved those down to a finally and check if they're initialized: 4c14716
I don't think Claude is correct in this case. The |
No description provided.