introduce uninterruptible frames #3189
Conversation
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
aconchillo
force-pushed
the
aleix/introduce-uninterruptible-frames
branch
from
03d4705 to
87668e5
Compare
There was a problem hiding this comment.
Shouldn't we also change FunctionCallsStartedFrame and FunctionCallCancelFrame? Otherwise, it feels like a race condition could happen.
There was a problem hiding this comment.
In the case of FunctionCallsStartedFrame we might want to notify right away that function calls are about to start . For FunctionCallCancelFrame it doesn't really matter. These two don't modify the context and don't trigger any inference. So, I didn't see a reason to change them.
There was a problem hiding this comment.
Yeah, I was wondering if we might have a race condition, but I think we’re okay. 👍
|
Tested the changes in this PR against the problem scenario described in #3175 and it resolves it! |
aconchillo
force-pushed
the
aleix/introduce-uninterruptible-frames
branch
from
87668e5 to
49b2b12
Compare
| # Put back UninterruptibleFrame frames into our process queue. | ||
| while not new_queue.empty(): | ||
| item = new_queue.get_nowait() | ||
| self.__process_queue.put_nowait(item) | ||
| new_queue.task_done() |
There was a problem hiding this comment.
Wouldn’t it be enough to simply do self.__process_queue = new_queue here ?
There was a problem hiding this comment.
No, because the process task is using the old queue still and we are not cancelling the task.
| # interruption). Instead we just drain the queue because this is | ||
| # an interruption. | ||
| self.__reset_process_task() | ||
| elif isinstance(self.__process_current_frame, UninterruptibleFrame): |
filipi87
left a comment
filipi87
left a comment
There was a problem hiding this comment.
Looks good. Nice improvement. Thank you for doing this. 🚀
Please describe the changes in your PR. If it is addressing an issue, please reference that as well.
In this PR we introduce the
UninterruptibleFramemixin. This mixin is used to mark certain data and control frames as uninterruptible. Sometimes we need to use ordered frames (data and control) but we don't want those frames to be discarded or cancelled if there's an interruption, this is what this new frame type is useful for.We also change the behavior of
FunctionCallInProgressFrameandFunctionCallResultFrame, bot have now changed from system frames to control and data frames, respectively. Both are set toUninterruptibleFrame.