Fix cli blockingioerror #11934
Fix cli blockingioerror #11934
Conversation
| try: | ||
| print(output) | ||
| except BlockingIOError: | ||
| print_blocking(output) | ||
|
|
||
|
|
||
| def print_success(*args: Any) -> None: |
There was a problem hiding this comment.
Actually we need a docstring here because the linter checks for any issues in the same module where the changes are being made (even if unrelated with the code changes). And we can't merge without making the linter happy 😅
| import sys | ||
| from typing import Any, Text, NoReturn | ||
|
|
||
| import rasa.shared.utils.io | ||
|
|
||
|
|
||
| def print_blocking(string) -> None: | ||
| """Save fcntl settings and restore them after print().""" |
There was a problem hiding this comment.
I'm unfamiliar with fcntl, could we expand the docstring a bit more about the need for this.
| def print_blocking(string) -> None: | ||
| """Save fcntl settings and restore them after print().""" | ||
| save = fcntl.fcntl(sys.stdout.fileno(), fcntl.F_GETFL) | ||
| new = save & ~os.O_NONBLOCK |
There was a problem hiding this comment.
could we add some comment above this line to explain what it's doing concisely.
| import sys | ||
| from typing import Any, Text, NoReturn | ||
|
|
||
| import rasa.shared.utils.io | ||
|
|
||
|
|
||
| def print_blocking(string) -> None: |
There was a problem hiding this comment.
Is it possible to add a unit test for this new function?
|
@trammell One more thing about |
|
@ancalita that is a very good point. I'm going to retract this PR and implement the changes discussed above. |
Proposed changes:
Add
print_blocking()and try/except block to manage blocking print errors in CLI.Fixes https://rasa-open-source.atlassian.net/browse/OSS-183
Status (please check what you already did):
black(please check Readme for instructions)