Skip to content
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

RFE: Notify on failure + tag github user(s) along with custom failure message #1911

Closed
lsm5 opened this issue Feb 14, 2023 · 9 comments · Fixed by #2182
Closed

RFE: Notify on failure + tag github user(s) along with custom failure message #1911

lsm5 opened this issue Feb 14, 2023 · 9 comments · Fixed by #2182
Assignees
Labels
area/config Related to the Packit's configuration. area/general Related to whole service, not a specific part/integration. complexity/single-task Regular task, should be done within days. gain/high This brings a lot of value to (not strictly a lot of) users. impact/low This issue impacts only a few users. kind/feature New feature or a request for enhancement.

Comments

@lsm5
Copy link

lsm5 commented Feb 14, 2023

As a packit-service user responsible for monitoring the packit jobs on the containers' team repos, I'd like to be notified whenever there's a packit failure on any PR, preferably via tagging my github id and a customizable message.

The current packit configuration docs only mention notification on build success, nothing for failure.

/cc @evgeni

@lsm5
Copy link
Author

lsm5 commented Feb 14, 2023

in case a custom copr project is used for builds, another option might be to tag the FAS accounts listed with builder perms on the copr.

@lachmanfrantisek lachmanfrantisek moved this from new to backlog in Packit Kanban Board Feb 16, 2023
@lachmanfrantisek
Copy link
Member

@lsm5 Thanks for creating the issue!

The outcome of today's team discussion:

  • use a comment for that (other possibilities have been considered)
  • provide a config option to set the content of the comment (and allow templating as a follow-up if needed)
    • if set, this feature is enabled
  • make sure we don't spam too much

@lsm5
Copy link
Author

lsm5 commented Feb 16, 2023

  • use a comment for that (other possibilities have been considered)

github comment is nice!

  • make sure we don't spam too much

can this be made configurable as well? Remind every x hours / days if no human response and / or still failing as set in the config?

Thank you!!

@lachmanfrantisek
Copy link
Member

can this be made configurable as well? Remind every x hours / days if no human response and / or still failing as set in the config?

To be really honest, we can't easily do any scheduled work without significant changes to Packit's architecture... (Packit now works on an event-based architecture.)

But I've searched through the GitHub marketplace and we might be able to combine Packit with some other GitHub action/app to do this. E.g. Packit can create a comment that will be picked by another action like this one to create reminders...

Sadly, I wasn't able to find anything generic capable of the whole workflow (because this might be handy not only for Packit statuses..;)

@martinpitt
Copy link

Adding a "me too". I am currently starting to establish some cross-project testing, see https://cockpit-project.org/blog/tmt-cross-project-testing.html . It doesn't scale well to tell these "other projects" to notify a set of people on failures, as people are notoriously bad at remembering to do that, and whom to contact.

This could possibly be modelled as an on: status GitHub workflow, but that would be hideously expensive. I think packit is in a much better position to do that -- it already sends an API request for updating the yellow to red or green -- that would be the correct place to also post a comment with such a pre-configured failure message. The most interesting part would be to @ the maintainers.

@lachmanfrantisek
Copy link
Member

@martinpitt thanks for describing your use case! Since also @evgeni mentioned this one to me yesterday as important for Forman projects, I am prioritising the card...;)

This can really bring Packit to projects independent of Fedora/Packit.

@mfocko mfocko moved this from ready-to-refine to refined in Packit Kanban Board Aug 17, 2023
@mfocko mfocko added complexity/single-task Regular task, should be done within days. impact/low This issue impacts only a few users. area/general Related to whole service, not a specific part/integration. gain/high This brings a lot of value to (not strictly a lot of) users. area/config Related to the Packit's configuration. kind/feature New feature or a request for enhancement. labels Aug 17, 2023
@lbarcziova lbarcziova self-assigned this Sep 11, 2023
@lbarcziova lbarcziova moved this from refined to in-progress in Packit Kanban Board Sep 11, 2023
lbarcziova added a commit to lbarcziova/packit that referenced this issue Sep 11, 2023
softwarefactory-project-zuul bot added a commit to packit/packit that referenced this issue Sep 12, 2023
Add failure_comment_message to notifications config

Related to packit/packit-service#1911
TODO:

 Update or write new documentation in packit/packit.dev.

I was also thinking about naming it only failure_comment, let me know WDYT.
RELEASE NOTES BEGIN
N/A
RELEASE NOTES END

Reviewed-by: František Lachman <[email protected]>
@martinpitt
Copy link

@lachmanfrantisek , @lbarcziova: We recently had a nice success story around that -- a podman PR broke some functionality, our reverse dependency tests caught it, it was debugged in an hour, and eventually the PR landed without the regression.

But I only found that because I regularly poll the podman PRs and just got lucky. As this isn't very reliable, I wanted to ask: Do you have a coarse time frame for this notification feature? I can probably write a hackjob GH workflow that does that polling in half a day or so, but I wonder if it's worth it.

Thanks!

@lbarcziova
Copy link
Member

@martinpitt happy to hear that!

Just yesterday I started to work on this feature, I believe is will be in production next Tuesday :)

@martinpitt
Copy link

@lbarcziova that sounds amazing, good luck! I'm happy to be a test guinea pig, of course. (However, I'll be on PTO this Thu/Fri, and possibly on Mon)

lbarcziova added a commit to lbarcziova/packit-service that referenced this issue Sep 12, 2023
If there is the failure_comment_message defined in the config, notify
users via comment on failure using the configured message. Comment
only once per commit SHA and job (utilising check of previous identical Packit comment).

Fixes packit#1911
lbarcziova added a commit to lbarcziova/packit-service that referenced this issue Sep 12, 2023
If there is the failure_comment_message defined in the config, notify
users via comment on failure using the configured message. Comment
only once per commit SHA and job (utilising check of previous identical Packit comment).

Fixes packit#1911
lbarcziova added a commit to lbarcziova/packit that referenced this issue Sep 12, 2023
Instead of having one value failure_comment_message, create a nested one
failure_comment.message so that failure_comment can be extended in the future
if needed. Related to packit/packit-service#1911
lbarcziova added a commit to lbarcziova/packit.dev that referenced this issue Sep 12, 2023
softwarefactory-project-zuul bot added a commit to packit/packit that referenced this issue Sep 12, 2023
Change configuration schema for failure comment

Instead of having one value failure_comment_message, create a nested one failure_comment.message so that failure_comment can be extended in the future if needed. Related to packit/packit-service#1911
RELEASE NOTES BEGIN
N/A
RELEASE NOTES END

Reviewed-by: František Lachman <[email protected]>
lbarcziova added a commit to lbarcziova/packit.dev that referenced this issue Sep 12, 2023
lbarcziova added a commit to lbarcziova/packit-service that referenced this issue Sep 13, 2023
If there is the failure_comment_message defined in the config, notify
users via comment on failure using the configured message. Check
for previous comment by Packit for duplication.

Fixes packit#1911
softwarefactory-project-zuul bot added a commit that referenced this issue Sep 13, 2023
Implement commenting on failure based on config

If there is the failure_comment_message defined in the config, notify users via comment on failure using the configured message. Comment only once per commit SHA and job (utilising check of previous identical Packit comment).
Fixes #1911
TODO:

 Update or write new documentation in packit/packit.dev. (packit/packit.dev#735)


RELEASE NOTES BEGIN
We have introduced a new configuration option notifications.failure_comment.message that enables notifying users on failure via a comment using the configured message.
RELEASE NOTES END

Reviewed-by: Laura Barcziová
Reviewed-by: František Lachman <[email protected]>
Reviewed-by: Martin Pitt
@github-project-automation github-project-automation bot moved this from in-progress to done in Packit Kanban Board Sep 13, 2023
lbarcziova added a commit to lbarcziova/packit.dev that referenced this issue Sep 20, 2023
softwarefactory-project-zuul bot added a commit to packit/packit.dev that referenced this issue Sep 20, 2023
Document notifications.failure_comment.message

Related to packit/packit-service#1911
Merge once in production.

Reviewed-by: František Lachman <[email protected]>
Reviewed-by: Laura Barcziová
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config Related to the Packit's configuration. area/general Related to whole service, not a specific part/integration. complexity/single-task Regular task, should be done within days. gain/high This brings a lot of value to (not strictly a lot of) users. impact/low This issue impacts only a few users. kind/feature New feature or a request for enhancement.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants