Don't use TextReceiveStream as a context manager #299
Conversation
With this change, CancelledErrors raised inside the loop will no longer cause the Stream to be closed; that is instead delayed to `kill_on_error()`.
| yield chunk | ||
| except BaseException: | ||
| log.exception("Exception raised while handling output from %s", desc) | ||
| raise |
There was a problem hiding this comment.
does stream need its await stream.aclose() in finally: or at the end of the try block?
There was a problem hiding this comment.
I don't think it really needs to be called at all. stream.aclose() just closes the underlying stream, which is here p.stdout, but that should be taken care of by the open_process() context manager.
|
@yarikoptic The tests are failing because the test Dandisets aren't validating in time. |
is it some new behavior of dandi-archive? then please alert dandi-archive team/file an issue. |
|
@yarikoptic It's an old behavior which has resurfaced. |
|
@yarikoptic Issue filed: dandi/dandi-archive#1365 |
|
ok, let's hope there is no side effects and give it a shot |
This is a guess at a possible way to deal with #293.
With this change, CancelledErrors raised inside the loop will no longer cause the Stream to be closed; that is instead delayed to
kill_on_error().