-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
readme example not async? #8
Comments
Had to try, and comment above is right. The original example is not async, or rather it runs asynch synchronously (though I could not for the life of me be able to find this out in a code review).
Additionally the example also caused DNS errors to appear after running it a few times. This is because the AsyncSession doesn't close properly (it just exists the program). Used versions:
|
I must partially agree and partially disagree with both comments. Neither one is asynchronous. The first example is not asynchronous for obvious reasons. The second example is still not asynchronous because in order to get from task #n to task #n+1, you have to wait for task #n-1 to finish and then wait for task #n to finish as well. While there would be some reduced wait time due to some asynchronicity, a properly asynchornous example would be one where the time to resolve all of the "promises" is no more than the time to resolve the longest-taking task, like JavaScript's While we are at it, a pull request open for 2 1/2 years? To fix the tiniest bit of documentation? Seriously??? I'm thinking the maintainer of the |
@FrankDMartinez Yeah, I opened this issue years ago. I don't really care about it anymore. However, I comment because I disagree with what you say: as soon as you call If you prefer a method more akin to |
@juliusbierk Sorry, I didn't mean you don't care. I meant the library maintainers don't care. I don't take seriously software which the maintainers don't care enough to keep up to date. |
Haha no offence taken! Well it happens to software all the time. :) |
The example in the readme file reads:
shouldn't it be
to be run async?
The text was updated successfully, but these errors were encountered: