MAINT Adds script to create or update issue when CI fails #21544
Conversation
| # TODO: Do we want to close the issue here? | ||
| print("Test has no failures!") |
There was a problem hiding this comment.
If there are not failures should we automatically close the issue if it exist?
There was a problem hiding this comment.
I think we could. If not we should at least update any existing issue.
There was a problem hiding this comment.
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.
agreed to move this to the upstream one
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.
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.
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.
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.
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.
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. |
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