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

Better subprocess listening experience #120

Merged
merged 2 commits into from
Nov 4, 2020

Conversation

davidlatwe
Copy link
Collaborator

Problem

Allzpark freeze when closing the application that it just launched, if Allzpark is using nerdvegas/rez instead of bleeding-rez.

The freeze was because nerdvegas/rez doesn't set universal_newlines=True when spawning shell by default, but bleeding-rez does.

So this PR will close #104 and close #119
(Sorry, should have notice that difference earlier)

Additional Improvement

  • Make subprocess encoding configurable.

    nerdvegas/rez enforced encoding="utf-8" if universal_newlines=True and encoding is not in kwarg. In some cases you may want to set the encoding you preferred ?

  • Add UnicodeDecodeError handler (Python>=3.6 only)

    Since Allzpark has the duty to listen to subprocess and displaying logs continually, adding a custom error handler that will firstly try to decode bytes with locale.getpreferredencoding, if still failing, use backslashreplace as final option.

bleeding-rez and nerdvegas/rez has a bit different shell
spawning setup.

This commit tries to makes people in both world have the
same subprocess experience when using Allzpark.

In addition, since Allzpark has the duty to listen to
subprocess and displaying logs continually, this commit
adds an error handler in case UnicodeDecodeError happens.
@mottosso
Copy link
Owner

Great detective work @davidlatwe. Feel free to merge this and #119 once it works for you.

@davidlatwe
Copy link
Collaborator Author

Merging this tomorrow :)

@davidlatwe davidlatwe merged commit 43655aa into mottosso:master Nov 4, 2020
@davidlatwe davidlatwe deleted the improve-subprocessing branch November 4, 2020 06:13
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