Suggestions for handling escape and csi characters #1888
Conversation
… to the SLURM, PBS and SGE scheduler
|
I get too many lines here for the |
aiida/transport/plugins/local.py
Outdated
| @@ -49,6 +48,10 @@ class LocalTransport(Transport): | |||
| # So I set the (default) limit to zero. | |||
| _DEFAULT_SAFE_OPEN_INTERVAL = 2. | |||
|
|
|||
| # List of escape and csi characters we forcefully remove from stdout | |||
| # and stderr | |||
| _EC_CHARACTERS = ['\x1b[H\x1b[J'] | |||
There was a problem hiding this comment.
aren't ANSI formatting sequences actual dual or multibyte sequences and not single characters?
So, something like:
_EC_CHARACTERS = [b'\x1b[H', ... ] # use bytes since those are ANSI, not unicode sequencesThere was a problem hiding this comment.
Yes, this indeed needs to be fixed, will update this.
| if input_string: | ||
| for entry in self._EC_CHARACTERS: | ||
| input_string = input_string.strip(entry) | ||
|
|
There was a problem hiding this comment.
and then here:
for entry in self._EC_CHARACTERS:
input_string = input_string.replace(entry, b'') # remove byte sequences, not single bytesmight be worthwhile to use something like colorama to get all the ANSI formatting sequences.
There was a problem hiding this comment.
Yes, I was unaware of that library. Seems to yield what we need (I know nothing of potential side effects at this point).
There was a problem hiding this comment.
but it might be better to use the ansifilter library directly instead
…_core into escape_handler_suggestions
…_core into escape_handler_suggestions
…er plugin to not set None if input is None
Pull Request Test Coverage Report for Build 3828
💛 - Coveralls |
|
Hi Espen, Would you be able to understand which string contains those characters? E.g. changing the tests to print the output_text (if those characters are found), to understand who is generating them? Once this is clear, we can understand what is the correct approach for fixing this. It might be a problem upstream (e.g. you have something set in your terminal to produce escape characters). Also, it would be useful if you could paste the output of a failed test, to see where the error shows up and again help in finding a more robust solution. Thanks! |
|
See also discussion #1887. Yes and no. I think we have to take a stand on the question of do we expect (or require) the users to run a clean system, e.g. no special colors or terminal setups. Once the answer to this question is settled, we can either disregard these issues (and rely on each users that have customs setups to fix it) or we have to find a more general solution. The characters come from the terminal and appear due to different user configs. The issue is currently only with the direct scheduler, since that analyses the output from the commands in a rather brute force manner. In addition, there is problems with the |
|
Hi @espenfl - so, for the issue of #1890, you can check my comment there - my position is that (since also e.g. sftp would break for instance) it's up to the user not to produce output or do other strange things in non-interactive shells - I put the solution in the issue and I think we should just document it. That, however, is only for SSH (I think). For #1887 and this PR, instead, apparently this happens for the local transport. Do you know, for the people who have this issue, if the guard I propose in #1890 would solve also this case? (If this the case, documenting it would be enough). |
|
I'm closing this, in favour of a different solution (#2132) |
Here follows a simple suggestion on how to strip escape and csi characters based on #1887. We do not want these characters as they upset downstream functionality. Currently unsure if we should also do this for the SshTransport. List needs to be updated with future issues or already known characters we want to eject.