fix(sdk): match reference behaviour for large error payloads#133
Conversation
ghost
requested review from
Changes: - When payloads are large, we checkpoint the error and return only failed. - When payloads are small, we return back the error fixes: #41
| ThreadPoolExecutor( | ||
| max_workers=2, thread_name_prefix="dex-handler" | ||
| ) as executor, | ||
| contextlib.closing(execution_state) as execution_state, |
There was a problem hiding this comment.
Adding here the context so that it always closes automatically.
|
|
||
| def close(self): | ||
| self.stop_checkpointing() |
There was a problem hiding this comment.
Ensures we close with the contextlib.
There was a problem hiding this comment.
nit: could we add a comment here, and the place that we use the contextlib.closing to point out we are stopping the ckeckpointing
|
|
||
| # Stop background checkpointing thread | ||
| execution_state.stop_checkpointing() | ||
| execution_state.create_checkpoint(success_operation, is_sync=True) |
There was a problem hiding this comment.
The reason we do create_checkpoint(success_operation, is_sync=True) over create_checkpoint_sync is because that was unwrapping the error. This made it appear like a user error, when in fact it was a checkpoint error.
The change in how we handle user errors made this bug visible.
|
I ran an integration test with this patch, confirmed that the error is handled correctly |
3 participants
Changes:
failed.
fixes: #41
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.