Skip to content

SRE-3930 ci: Retry SCM notifications without failing the build - #532

Merged
ryon-jensen merged 8 commits into
masterfrom
osalyk/SRE-3930_improve-github-notify
Sep 23, 2026
Merged

ryon-jensen merged 8 commits into
masterfrom
osalyk/SRE-3930_improve-github-notify

Conversation

@osalyk

@osalyk osalyk commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Signed-off-by: Oksana Salyk <oksana.salyk@hpe.com>
@osalyk
osalyk requested review from grom72 and janekmi September 2, 2026 12:37
Comment thread vars/scmNotify.groovy
Signed-off-by: Oksana Salyk <oksana.salyk@hpe.com>
@osalyk
osalyk marked this pull request as ready for review September 3, 2026 14:09
Comment thread src/test/groovy/testScmNotify.groovy
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy
Comment thread src/test/groovy/testScmNotify.groovy
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Signed-off-by: Oksana Salyk <oksana.salyk@hpe.com>
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Signed-off-by: Oksana Salyk <oksana.salyk@hpe.com>
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Comment thread src/test/groovy/testScmNotify.groovy Outdated
Signed-off-by: Oksana Salyk <oksana.salyk@hpe.com>
janekmi
janekmi previously approved these changes Sep 17, 2026
grom72
grom72 previously approved these changes Sep 18, 2026
Comment thread vars/scmNotify.groovy

try {
scmNotifyTrusted(config)
} catch (Exception e) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What would happen if the build is canceled by user during the sleep? Do we need something like this?

} catch (org.jenkinsci.plugins.workflow.steps.FlowInterruptedException e) {
    throw e
} catch (InterruptedException e) {
    throw e
} catch (Exception e) {
    echo "ERROR: could not notify GitHub after ${notifyAttempt} attempts " +
        "(${e.message}); continuing because status notification is non-fatal."
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. I added explicit handling for InterruptedException. Since Jenkins' FlowInterruptedException is a subclass of it, this also covers user cancellation during sleep without requiring Jenkins-specific imports in the unit test.

Signed-off-by: Oksana Salyk <oksana.salyk@hpe.com>
@osalyk
osalyk dismissed stale reviews from grom72 and janekmi via 1ce3dec September 22, 2026 10:19
Signed-off-by: Oksana Salyk <oksana.salyk@hpe.com>
@ryon-jensen
ryon-jensen merged commit c525ea2 into master Sep 23, 2026
4 of 5 checks passed
@ryon-jensen
ryon-jensen deleted the osalyk/SRE-3930_improve-github-notify branch September 23, 2026 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants