-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add support for CDEvents notifications #18853
base: main
Are you sure you want to change the base?
Conversation
/label release-note/new-feature |
Signed-off-by: Andrea Frittoli <[email protected]>
Signed-off-by: Andrea Frittoli <[email protected]>
@afrittoli Hi, thanks for your work! Please update to resolve the conflict. |
Codecov Report
@@ Coverage Diff @@
## main #18853 +/- ##
=======================================
Coverage 44.71% 44.71%
=======================================
Files 235 235
Lines 13083 13083
Branches 2671 2671
=======================================
Hits 5850 5850
- Misses 6938 6939 +1
+ Partials 295 294 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
// Only TopicPullArtifact is supported for now. | ||
// Other event types are not striclty a failure (they are missing by design), | ||
// but we still report them upstream as unknown types | ||
if he.EventType != event.TopicPushArtifact { |
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.
Yep, I agreed that other event types are not a failure case, user can subscribe other event types from UI but can only receive PUSH events, IMO the better solution is that extend the capability to support limit the event types.
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
This PR was closed because it has been stalled for 30 days with no activity. If this PR is still relevant, please re-open a new PR against main. |
I would like to continue working on this - is it an option to re-open? If not I will open a new PR. |
@afrittoli reopen :) please excuse the automation :D |
Comprehensive Summary of your change
Introduce a new notifier format "CDEvents", as described in the CDEvents proposal.
The CDEvents formatter, for now, supports only
PUSH_ARTIFACT
events.If other events are selected in the UI, they raise an error in the logs and no event is sent.
The PR is made of two commits:
Issue being fixed
Fixes #community/225
Please indicate you've done the following: