Skip to content
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

Use asyncio interface for subprocess calls #53

Closed
whyitfor opened this issue Oct 4, 2022 · 2 comments
Closed

Use asyncio interface for subprocess calls #53

whyitfor opened this issue Oct 4, 2022 · 2 comments
Assignees
Labels
good first issue Good for newcomers hacktoberfest Participate in hacktoberfest!

Comments

@whyitfor
Copy link
Contributor

whyitfor commented Oct 4, 2022

Many components use subprocess to make calls to external programs via the command line. These are currently blocking, non-asynchronous calls.

The asyncio.create_subprocess_exec and asyncio.create_subprocess_shell functions represent an asyncio-compatible API for making subprocess calls that don't block the event loop. We should replace existing subprocess calls in OFRAK with async calls via this API.

Which files would be affected?
Any file that uses a subprocess call. (ofrak_patch_maker should not be updated since it is a synchronous package).

Does the proposed maintenance include non-doc string functional changes to the Python code?
Yes (see above)

Are you interested in implementing it yourself?
No -- this is a great first contributor issue!

@marczalik
Copy link
Contributor

@rbs-jacob Should this change be made for subprocess calls inside of fixtures and test cases too?

@rbs-jacob
Copy link
Member

@rbs-jacob Should this change be made for subprocess calls inside of fixtures and test cases too?

That seems less important to me. The main issue is that the subprocess calls currently block the event loop to unpack a filesystem, for example. This lag directly affects a user by fully blocking OFRAK concurrency for an inherently asynchronous operation, whereas the async concurrency is less meaningful in the tests. This is because the tests are either run fully serially, or explicitly concurrently using multiprocessing via pytest.

Does that clarify?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers hacktoberfest Participate in hacktoberfest!
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

4 participants