-
Notifications
You must be signed in to change notification settings - Fork 167
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
feat(controller): record verification history Stage #1645
Conversation
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
✅ Deploy Preview for docs-kargo-akuity-io ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1645 +/- ##
==========================================
+ Coverage 44.14% 44.15% +0.01%
==========================================
Files 199 199
Lines 12728 12790 +62
==========================================
+ Hits 5619 5648 +29
- Misses 6868 6900 +32
- Partials 241 242 +1 ☔ View full report in Codecov by Sentry. |
I think that's the right move. Stages have a stack of FreightReferences in their history and the top of that stack always matches what's in the CurrentFreight field. So technically, the CurrentFreight field is redundant... but it was useful to be able to see what the current Freight was without having to know with certainty whether the history used stack or queue semantics... I think it's the same thing here with the verifications. |
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.
Test drove and it works very smoothly!
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Fixes: #1638
This adds a
VerificationHistory
field to theFreightReference
of a Stage, containing a list of the last 10 verification attempts. Technically, I would say this addition makes theVerificationInfo
field obsolete. But I kept it for backwards compatibility reasons.To ensure the AnalysisRun reference for a verification item is always kept around whenever we can (either in the error message, or as
AnalysisRunReference
), aborting an AnalysisRun will now maintain the reference information.