Draft
Conversation
- Add deduplicationEnabled parameter to RecordEvent and RecordWarning - RecordReady always uses deduplication - Add comprehensive tests for all deduplication scenarios
|
|
||
| // Check if any existing event matches the type, reason, and message | ||
| for _, event := range eventList.Items { | ||
| if event.Type == eventType && event.Reason == reason && event.Message == message { |
Contributor
There was a problem hiding this comment.
Let me think about this more deeply because I think we might want some additional complexity here or still require client to do some work. As an example if I am marking my object as ready in an event, I want to capture what metadata.generation the object is ready at. If the current generation is 5 but we marked it ready for generation 1, the event is eventually not useful.
More and more it feels like its up to the client to determine what the dedup logic is so if we are going to add to the sdk, I just want to think a bit more deeply. Curious on your thoughts as well
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💸 TL;DR
This PR adds optional dedupe logic to the events package. If enabled, It looks at the existing events on the Resource, and if an identical event exists, it does not record the event.
🧪 Testing Steps / Validation
I've just tested using my brain and the unit tests - I'd love advice for how I can test this against a real cluster.
✅ Checks