-
Notifications
You must be signed in to change notification settings - Fork 20
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
several small improvements #177
Conversation
We were generating it for the DAG op but never used if it was empty Also use litter to dump the stage contents with a cleanup so it doesnt show empty fields Signed-off-by: Itxaka <[email protected]>
Its too heavy and always the same info (its run on init) so having it repeated time and time can get pretty bad. This makes the logs much nicer and cleaner Signed-off-by: Itxaka <[email protected]>
Signed-off-by: Itxaka <[email protected]>
Signed-off-by: Itxaka <[email protected]>
Otherwise it gets reproted several times Signed-off-by: Itxaka <[email protected]>
var errs error | ||
for _, p := range e.conditionals { | ||
if err := p(e.logger, stage, fs, console); err != nil { | ||
e.logger.Warnf("(conditional) Skip '%s' stage name: %s", | ||
err.Error(), stage.Name) | ||
err.Error(), stageName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This diff with the actual Apply method was causing the DAG to be created properly with names generated if the stage had no name, but then on the log it will still shop as empty.
b, _ := json.Marshal(stage) | ||
e.logger.Debugf("Stage: %s", string(b)) | ||
litter.Config.HideZeroValues = true | ||
e.logger.Debugf("Stage: %s", litter.Sdump(stage)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this dumps the actual stages in a much more nicer way. It shows no empty values, so only things that are gonna be run and its in a nice structured way which makes it simpler to understand visually.
l.Debug(string(data)) | ||
litter.Config.HideZeroValues = true | ||
litter.Config.HidePrivateFields = true | ||
l.Trace(litter.Sdump(&system)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as the other but put this on trace level. We dont usually need this as part of the debug part and its very ugly,big and pollutes the log with a lot of useless, repeated info.
Co-authored-by: Ettore Di Giacinto <[email protected]>
Co-authored-by: Ettore Di Giacinto <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good!
see commits for explanation
Signed-off-by: Itxaka [email protected]