-
-
Notifications
You must be signed in to change notification settings - Fork 26k
MAINT Adds script to create or update issue when CI fails #21544
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
Conversation
# TODO: Do we want to close the issue here? | ||
print("Test has no failures!") |
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.
If there are not failures should we automatically close the issue if it exist?
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.
I think we could. If not we should at least update any existing issue.
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.
I don't know, what would be the false positive rate on auto close?
/cc @jjerphan as you might be interested in an example of usage of the GitHub API. |
The report is really great. Maybe we could generalize this idea of analyzing the CI output by maintaining a high level, user friendly short summary of the main problems of a given PRs (e.g. at the bottom of the PR description or in a continuously updated comment maintained by the bot) without having to go through the complex Azure Pipelines UI and verbose logs. |
Could you please also update the CI section of the maintainers doc to mention what this bots does? In particular if you need particular account credentials it would be great to make it explicit how those are managed and where the secret tokens are configured (if any). |
I am +1 for this feature. I really find the clicking pattern to go to the CI quite intimidating for newcomers. |
azure-pipelines.yml
Outdated
@@ -144,6 +144,7 @@ jobs: | |||
BLAS: 'openblas' | |||
COVERAGE: 'false' | |||
BUILD_WITH_ICC: 'false' | |||
CREATE_ISSUE_ON_TRACKER: 'true' |
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.
agreed to move this to the upstream one
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.
LGTM. Thanks @thomasjpfan for setting this up!
build_tools/azure/posix.yml
Outdated
versionSpec: '3.9' | ||
displayName: Place Python into path to update issue tracker | ||
condition: and(succeededOrFailed(), eq(variables['CREATE_ISSUE_ON_TRACKER'], 'true'), | ||
in(variables['Build.Reason'], 'Schedule', 'IndividualCI')) |
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.
does this mean we run this for individual CI runs? I thought we don't want that, since we only have this for nightly scipy-dev runs
title = f"⚠️ CI failed on {args.ci_name} ⚠️" | ||
|
||
|
||
def get_issue(): |
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.
would it be possible to check the dependency and scikit-learn versions of the open issue against the one in the current CI, and then only return that issue if they match?
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.
It is possible, but I think it makes everything a bit more complicated. If we check the dependencies and scikit-learn version, then the scipy-dev
config may have multiple open issues. For example, if the scipy-dev config
has updated dependencies, it would open another issue. This happens quite frequently because scipy-dev
uses the nightly builds. In these cases, we either let the CI close the old issues, or we close it ourselves.
I prefer the current implementation since it has a one-to-one mapping between a [scipy-dev]
and an open issue.
- If
[scipy-dev]
is passing -> close issue connected to[scipy-dev]
- If
[scipy-dev]
is failing -> update issue connected to[scipy-dev]
or create a new issue if there isn't an open issue connected to[scipy-dev]
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.
Fair enough, we can let it be and see how it works. I'd also be happy if we had a ping to @core-devs
to the first post of each issue. We can then unsubscribe if we want.
print("Test has no failures!") | ||
issue = get_issue() | ||
if issue is not None: | ||
print(f"Closing issue #{issue.number}") |
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.
I think this is where the version check would be useful, so that we only close automatically if the old one used the same versions
I'll let @ogrisel do another check and merge if all good. |
IIRC, @ogrisel is off this week. |
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.
LGTM, let's give it a shot. I will then do a follow-up PR to move the PyPy job as a scheduled job with this option enabled.
Reference Issues/PRs
Related to #21286
What does this implement/fix? Explain your changes.
This PR automatically creates an issue or update an existing issue when a scheduled CI fails. Here is what it looks like: thomasjpfan#92
Any other comments?
The script is CI agnostic and should be easy to use with GitHub Actions.
CC @ogrisel