Skip to content

Conversation

@tiangolo
Copy link
Contributor

@tiangolo tiangolo commented Apr 2, 2019

Make StreamingResponse compatible with normal iterators.

Continuing from #457, #457 (comment)


I also removed the mention to iterator_to_async from the docs, only mention that it supports both async and normal generators.

I'd also be interested in if there's any nice ways around we could use something like this and drop aiofiles [...]

Let's see what do you think about this one and then I'll take a stab at that 😄

raise _StopSyncIteration


async def iterator_to_async(iterator: Iterator) -> AsyncGenerator:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about iterate_in_threadpool to mirror run_in_threadpool.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea. I struggled a lot to name it and wasn't quite convinced either.

@lovelydinosaur
Copy link
Contributor

Do we strictly need the _StopSyncIteration indirection?

@lovelydinosaur
Copy link
Contributor

Can we not catch StopIteration directly?

@tiangolo
Copy link
Contributor Author

tiangolo commented Apr 2, 2019

StopIteration is used internally by coroutines, it was used before to yield values, here's a short summary in the PEP: https://www.python.org/dev/peps/pep-0492/#why-stopasynciteration

I can think of an alternative using classes, based on this example: https://www.python.org/dev/peps/pep-0492/#example-2, would you prefer that?

@tiangolo
Copy link
Contributor Author

tiangolo commented Apr 2, 2019

Because StopIteration was used to yield values, catching it directly in iterate_in_threadpool is not possible, it is converted to a RuntimeError.

Actually, now that I think of it, using a class would still need the indirection, as we need an exception that is propagated up/out even when run inside of run_in_threadpool.

@lovelydinosaur
Copy link
Contributor

👍 Released as prerelease version 0.12.0b3

@tiangolo
Copy link
Contributor Author

tiangolo commented Apr 3, 2019

Awesome! Thanks! 🎉 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants