-
Notifications
You must be signed in to change notification settings - Fork 6
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
async stuff #11
base: master
Are you sure you want to change the base?
async stuff #11
Conversation
rickshaw/node_server.py
Outdated
from asyncio.subprocess import create_subprocess_exec | ||
from argparse import ArgumentParser | ||
|
||
def make_parser(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PEP8 two blank lines before functions. All throughout this file
rickshaw/node_server.py
Outdated
pending_tasks.append(sim_task) | ||
i += 1 | ||
if len(pending_tasks) > 0: | ||
done, pending_tasks = await asyncio.wait(pending_tasks, return_when=concurrent.futures.FIRST_COMPLETED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PEP8 line over 80 chars
Generally looks good, but we probably also need some docs for this. Also, maybe we should remove doctr for the PR here and we should probably run the actual tests on circleci |
Maybe CI should be in a different PR though? |
Yeah, CI should probably go in a separate PR. Documentation is the right call though. |
Ok, let me know when it is added! |
No description provided.