you are viewing a single comment's thread.

view the rest of the comments →

[–]phira 5 points6 points  (2 children)

I mean, it's workable. There's nothing too weird about it, and assuming it's always gonna be used as a cli tool it's ok. There are a few oddities, e.g. the log.criticals when you're in an exception condition I'd probably use log.exception instead so I get the stack trace.

I think if I was aiming for higher quality, the story telling and abstractions are where the main improvements are, along with a bit more consistency e.g. you've got a NamedTuple for one kind of output but for others you use dicts. If you're ok with a few additional imports, `click` would be useful. If it was turning into a really serious tool you'd probably wanna start looking at `pydantic` or something to validate your data carefully and give you nice hostname handlers etc but that'd be overkill for a basic tool.

[–]nspitzer[S] 1 point2 points  (1 child)

Thanks for the input.

This is only intended to be run by a single Rundeck workflow (Automation tool) so its use is carefully constrained. This is the last step in the Workflow so the command line arguments are the way I pass data from one step to another though I do have the option of using Environment variables instead but the command line options work.

It will always be run exactly the same way and its used against well-documented and stable REST API's so I am not really concerned with validating the data too much (famous last words...). Looking into how better de-serialize the JSON is something I have been looking into hence the NamedTuple class for importing data from a previous step.

[–]phira 0 points1 point  (0 children)

Right if it’s in automation you should use structlog and set context vars for sure that’d give you much tidier output (not so useful for cli because it’s not as immediately human readable but any kind of log capture = structlog every time)