Real time output for tool deploy scripts

[mickmcgrath13]

Description

Uses subprocess.Popen instead of subprocess.run and uses subprocess.PIPE and process.poll() to output the subprocess execution line-by-line

Fixes #320

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update

How Has This Been Tested?

With a local ops repo with helm charts. Ran the 2.1.0 docker container locally and mounted the ops repo and the BitOps code to the proper locations within the container.

Logs
..were output in real time

Test Configuration:

• BitOps image tag used: 2.1.0
• Hardware: MacOS
• Tool: helm, terraform

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation

[mickmcgrath13]

this is probably not necessary, but I left it in because something about the implementation seems off (see previous comment)

[mickmcgrath13]

This was strange to me.. If we do both stderr and stdout like:

stdout=subprocess.PIPE,
stderr=subprocess.PIPE

..the below polling doesn't seem to work.
even if stderr is one of:

• err (i.e. a file opened with `with open(error_file, "wb") as err:
• subprocess.PIPE
• subprocess.STDOUT

..then the weird thing is that if we do just stdout=subprocess.PIPE,, we do not get errors, but if we do stderr=subprocess.PIPE, we get both stdout and stderr real time output 🤷

[arm4b]

I understand the easy hack idea here was just combining two streams into one if we don't need to process these streams separately.

I see where it comes from. Based on https://docs.python.org/3/library/subprocess.html#using-the-subprocess-module

If capture_output is true, stdout and stderr will be captured. When used, the internal Popen object is automatically created with stdout=PIPE and stderr=PIPE. The stdout and stderr arguments may not be supplied at the same time as capture_output. If you wish to capture and combine both streams into one, use stdout=PIPE and stderr=STDOUT instead of capture_output.

Can you try the following? No while True loop needed then.

process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
for combined_output in process.stdout:
    print(combined_output)

It worked for me in quick console tests. The difference is that it reads stdout (which has combined output) instead of stderr.

That would be at least more intuitive and follow the official docs.
If works, we can reference that documentation URL in code comments then and call the mystery as solved.

[mickmcgrath13]

@armab #337 (review)

[mickmcgrath13]

also, the quote from the link above is for subprocess.run not subprocess.Popen

[mickmcgrath13]

@armab @PhillypHenning I could use assistance figuring out what this lint error means:
https://github.com/bitovi/bitops/actions/runs/3323834010/jobs/5494675369#step:6:78

[arm4b]

black as a code formatter was added in #297 with some documentation here: https://bitovi.github.io/bitops/development/development/#python-style-guide
black for Python is like go fmt for Golang.

@mickmcgrath13 It hints at the code diff here.

You can reformat the code with black locally:

pip3 install black
black scripts/

Or can add it to VSCode so it reformats .py files on save automatically:
https://dev.to/adamlombard/how-to-use-the-black-python-code-formatter-in-vscode-3lo0

[mickmcgrath13]

@armab I tried this. see comment

[arm4b]

Thanks for trying it.

Screen.Recording.2022-10-27.at.14.54.47.mov

I will check what's missing end to end when applying this draft to our script.

[mickmcgrath13]

this makes me think that it might be a docker or OS (base docker image) problem 🤔

[arm4b]

@mickmcgrath13 Tried editing the original Docker container inline with that version of deploy_plugins.py and the live output worked, outputting terraform results line-by-line as we get it.

Screen.Recording.2022-10-27.at.19.44.40.mov

Make sure there's no cache involved. Worst case, we can build the temporary testing Docker images later to try it again.

[mickmcgrath13]

we tried this in the same container I'm using:

import subprocess
command = ["ping", "-c", "3", "1.1"]
process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, universal_newlines=True)
for combined_output in process.stdout:
    print(combined_output)

Did this by exec'ing into the container, running python3 interactive terminal, and it worked as expected.

[mickmcgrath13]

adding -e PYTHONUNBUFFERED=1 \ to the bitops container call seemed to help

[arm4b]

Nice! Looks like PYTHONUNBUFFERED=1 is another ENV var to hardcode in the base Dockerfile as an addition to

bitops/Dockerfile

Line 3 in e6f705d

ENV DEBIAN_FRONTEND=noninteractive