-
Notifications
You must be signed in to change notification settings - Fork 116
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
New Monitor class with medium verbosity level #1995
Comments
@aesteve-rh Generally we've (now) been doing this higher up in the stack in the things that use I've previously taken a look at prettifying (some) output and logging with Let me tag @mvo5 since he has done (most of) the work on the progress reporting things. As an aside; are you often executing |
I work on automotive, so I rely on One could of course argue that we could beautify the output on Let me cc @alexlarsson to this conversation. |
Right so to address your middle concern directly, we already do progress bars and reporting in the projects that are using
For Personally I'm fine with reviewing a new monitor that does progress reporting inside |
If you feel that the right place for this is the caller of osbuild, then we can add the code to a-i-b to fake some progress bars, but it seems like osbuild is the best place to do it correctly, as it knows exactly what work it is preparing to do. |
Having I could go to That said, I have no problem in implementing that on aib, as @alexlarsson said. Just wanted to raise the discussion and reach an agreement. |
Sorry for coming in a bit late. We build this json-seq based progress so that our consumers are flexible in how represent the progress/monitoring (one of our targets for this is also the webservice) and should have all the information that the monitor in python also has. But if you really just want a tiny "PrettyMontior" with a bit of rich that seems fine to me, please make it robust against import failures (rich is afaik not available on all our target OSes so we cannot add it as a dependency everywhere) and please make sure that in case of failure the full stage output is shown for debug purposes and please add tests. Having said this, just using something like https://github.com/osbuild/osbuild/blob/main/tools/osbuild-json-seq-progress-example-renderer will work today, something like:
or something (with nice rich sprinckled in) like: import json
import sys
class SmallProgressRenderer:
def __init__(self, inf):
self._inf = inf
def _read_json_seq_rec(self):
while True:
line = self._inf.readline()
if not line:
return None
try:
payload = json.loads(line.strip("\x1e"))
except json.JSONDecodeError:
print(f"WARN: invalid json: {line}")
continue
return payload
def render(self):
while True:
js = self._read_json_seq_rec()
if js is None:
return
ctx = js["context"]
if ctx.get("origin") == "osbuild.monitor":
print(js.get("message"))
pipeline_result = js.get("result")
if pipeline_result:
print(js["message"])
if not pipeline_result['success']:
print("pipeline output:")
print(pipeline_result["output"])
if __name__ == "__main__":
prg = SmallProgressRenderer(sys.stdin)
prg.render() The "osbuild.monitor" origin is already kinda reporting this high level stuff it might be enough for your specific use-case. Progressbar is of course also possible, see the above example. |
So for builds, osbuild has two monitors that I could see,
NullMonitor
, andLogMonitor
. While this is nice, it would be helpful to tune down a bit the verbosity of theLogMonitor
so that we only print the name of the stage and some minimal information, ideally reduce it to one line per stage.The reasoning behind this is to make it easier to spot fails/issues, as building an image with many stages turns to be too noisy to realize an error is printed (unless the build actually failed, of course).
Ideally, we could use rich or a similar library to beatify it and add ticks to indicate whether a stage succeeded or not. Maybe even a status bar that reaches 100% when no stages are left.
I am not really seeking for someone to do this, but rather checking on the general maintainers' opinion/reception before I start implementing.
The text was updated successfully, but these errors were encountered: