Skip to content

Implement commenting on failure based on config#2182

Merged
softwarefactory-project-zuul[bot] merged 1 commit intopackit:mainfrom
lbarcziova:notify-on-failure
Sep 13, 2023
Merged

Implement commenting on failure based on config#2182
softwarefactory-project-zuul[bot] merged 1 commit intopackit:mainfrom
lbarcziova:notify-on-failure

Conversation

@lbarcziova
Copy link
Member

@lbarcziova lbarcziova commented 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 #1911

TODO:


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

@softwarefactory-project-zuul

This comment was marked as outdated.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/8bf22aa4579945e9adaffe388d3fb47a

✔️ pre-commit SUCCESS in 1m 53s
✔️ packit-service-tests SUCCESS in 2m 38s
✔️ packit-service-tests-openshift SUCCESS in 13m 09s

Copy link
Member

@lachmanfrantisek lachmanfrantisek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

return

formatted_message = configured_message.format(
commit_sha=self.db_project_event.commit_sha
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I quite like the job_name variable, or, maybe some links might be requested. But we can freely stay with this and add on request later on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it since users can define their custom messages per-job which can replace this, but I am not strongly against

maybe @martinpitt could help us with the feedback here: would you be interested in Packit providing some other variables besides commit_sha (see the docs in preparation) that you could utilise in the configuration of the custom failure messages? :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The big enabler here is to get a configurable list of people to ping, all the details can then be examined on the PR page.

Personally I liked the first version (i.e. just the first commit) a bit better: it's (mildly) useful to have the job name in the message, and it is IMHO much better to not interpolate configured_message -- adding variables to the text sounds like a recipe for bugs.

But it's not a strong opinion -- I'd be happy either way (and really looking forward to this!)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the feedback! I can see that as @lachmanfrantisek wrote before, some users would like to have this completely customizable and by providing the commit_sha, actually, the duplication of the comment in the PR can be configured as well: when {commit_sha} is not present in the configured message, there will be only one Packit comment per PR and when it will be present, Packit will place a new comment for each failure for different commit

I would probably test it in staging in the current form and we can still easily tweak the content of the comment if needed

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/b7bff307b1994794b2b32a27be440730

✔️ pre-commit SUCCESS in 1m 46s
✔️ packit-service-tests SUCCESS in 2m 11s
✔️ packit-service-tests-openshift SUCCESS in 17m 42s

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
@lbarcziova lbarcziova added the mergeit Merge via Zuul label Sep 13, 2023
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/20c7871c958047eb81de58e1c5dc46b6

✔️ pre-commit SUCCESS in 1m 44s
✔️ packit-service-tests SUCCESS in 1m 55s
✔️ packit-service-tests-openshift SUCCESS in 12m 14s

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).
https://softwarefactory-project.io/zuul/t/packit-service/buildset/549ac3f2a1f84995bff2cedceb23f2aa

✔️ pre-commit SUCCESS in 1m 44s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 3f0fe2d into packit:main Sep 13, 2023
@martinpitt
Copy link

Amazing, thank you! Can't wait to trying it out next week! 💯

@lbarcziova lbarcziova deleted the notify-on-failure branch September 13, 2023 07:07
@lbarcziova
Copy link
Member Author

Tested here.

@lsm5
Copy link

lsm5 commented Sep 13, 2023

@lbarcziova This is awesome. podman team will be relieved to have this so they won't have to manually tag me going forward :D

lbarcziova added a commit to lbarcziova/packit-service that referenced this pull request Sep 25, 2023
When creating issues/commenting in issue_repository, use the configured
notifications.failure_comment.message in downstrem jobs.
Followup of packit#2182
lbarcziova added a commit to lbarcziova/packit-service that referenced this pull request Sep 26, 2023
When creating issues/commenting in issue_repository, use the configured
notifications.failure_comment.message in downstrem jobs.
Followup of packit#2182
softwarefactory-project-zuul bot added a commit that referenced this pull request Sep 26, 2023
…bs (#2199)

Use configured failure message as suffix in comment for downstream jobs

When creating issues/commenting in issue_repository, use the configured notifications.failure_comment.message in downstrem jobs. Followup of #2182

RELEASE NOTES BEGIN
You can now configure notifications.failure_comment.message also for downstream jobs, where the configured message will be used as an extension of the comment added by default by Packit.
RELEASE NOTES END

Reviewed-by: František Lachman <flachman@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mergeit Merge via Zuul

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

4 participants