Skip to content

async stuff#11

Open
FlanFlanagan wants to merge 2 commits into
cyclus:masterfrom
FlanFlanagan:node_serv
Open

async stuff#11
FlanFlanagan wants to merge 2 commits into
cyclus:masterfrom
FlanFlanagan:node_serv

Conversation

@FlanFlanagan

Copy link
Copy Markdown
Member

No description provided.

Comment thread rickshaw/node_server.py
from asyncio.subprocess import create_subprocess_exec
from argparse import ArgumentParser

def make_parser():

Copy link
Copy Markdown
Member

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

Comment thread rickshaw/node_server.py
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)

Copy link
Copy Markdown
Member

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

@scopatz

scopatz commented Nov 1, 2017

Copy link
Copy Markdown
Member

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

@scopatz

scopatz commented Nov 1, 2017

Copy link
Copy Markdown
Member

Maybe CI should be in a different PR though?

@FlanFlanagan

Copy link
Copy Markdown
Member Author

Yeah, CI should probably go in a separate PR. Documentation is the right call though.

@scopatz

scopatz commented Nov 8, 2017

Copy link
Copy Markdown
Member

Ok, let me know when it is added!

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