refactor: skip reconciliation for some presets #17595
Conversation
| @@ -310,6 +310,15 @@ func (c *StoreReconciler) ReconcilePreset(ctx context.Context, ps prebuilds.Pres | |||
| return nil | |||
| } | |||
|
|
|||
| // Nothing has to be done. | |||
| if !ps.Preset.UsingActiveVersion && actions.IsNoop() { | |||
There was a problem hiding this comment.
@dannykopping Do we need !ps.Preset.UsingActiveVersion? I think we can skip reconciliation for all presets if actions.IsNoop() == true?
There was a problem hiding this comment.
I guess, yeah, but if the preset is part of an inactive version then we shouldn't be logging anything for that; for active presets which have no operations to perform, that's at least diagnostically useful.
Consider an installation that's been using prebuilds for a while and they have >100 inactive preset versions; that'll log a tonne for no value.
There was a problem hiding this comment.
okay, let's keep it as it is
There was a problem hiding this comment.
I mean here we log with debug level which is probably okay?
// Nothing has to be done.
if !ps.Preset.UsingActiveVersion && actions.IsNoop() {
logger.Debug(ctx, "skipping reconciliation for preset - nothing has to be done",
slog.F("template_id", ps.Preset.TemplateID.String()), slog.F("template_name", ps.Preset.TemplateName),
slog.F("template_version_id", ps.Preset.TemplateVersionID.String()), slog.F("template_version_name", ps.Preset.TemplateVersionName),
slog.F("preset_id", ps.Preset.ID.String()), slog.F("preset_name", ps.Preset.Name))
return nil
}and then exit early without additional logs with info/warn log level
Or you wanted to delete debug log as well?
There was a problem hiding this comment.
Nope we can keep the debug log 👍
No description provided.