Skip to content

Live stdout and stderr#55

Open
mxmrlv wants to merge 1 commit into
diyan:masterfrom
mxmrlv:master
Open

Live stdout and stderr#55
mxmrlv wants to merge 1 commit into
diyan:masterfrom
mxmrlv:master

Conversation

@mxmrlv

@mxmrlv mxmrlv commented May 31, 2015

Copy link
Copy Markdown

I've added some really basic support for writing to streams. This enables to keep live track on the stdout and stderr of the remote machine. I've also added a test to test this functionality.

Also, i wanted to propose the following code change; instead of:

    stdout = stderr = ''
    return_code = -1
    for stream_node in stream_nodes:
        if stream_node.text:
            content = str(base64.b64decode(stream_node.text.encode('ascii')))
            if stream_node.attrib['Name'] == 'stdout':
                if out_stream:
                    out_stream.write(content)
                stdout += content
            elif stream_node.attrib['Name'] == 'stderr':
                if err_stream:
                    err_stream.write(content)
                stderr += content
    ...
    return stdout, stderr, return_code, command_done

I believe it would be tidier that way:

    out_string = {
        'stdout': '',
        'stderr': ''
    }
    out_stream = {
        'stdout': out_stream,
        'stderr': err_stream
    }
    return_code = -1
    for stream_node in stream_nodes:
        if stream_node.text:
            content = str(base64.b64decode(stream_node.text.encode('ascii')))            
            out_string[stream_node.attrib['Name']] += content
            if out_stream[stream_node.attrib['Name']]:
                out_stream[stream_node.attrib['Name']].write(content)
    ...
    return out_string['stdout'], out_string['stderr'], return_code, command_done

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.2%) to 61.16% when pulling 3efe4c3 on max-orlov:master into c9ce62d on diyan:master.

@cloneluke

Copy link
Copy Markdown

any reason for not merging this PR?

@kootenpv

Copy link
Copy Markdown

I was also looking for this. Please consider the merge.

@ltamaster

Copy link
Copy Markdown

+1

@pawelros

Copy link
Copy Markdown

+1

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.

6 participants