CI Add workflow to update lock files by lesteve · Pull Request #27622 · scikit-learn/scikit-learn

lesteve · 2023-10-19T15:40:02Z

Reference Issues/PRs

What does this implement/fix? Explain your changes.

This adds a github workflow to manually update the lock files. Right now this will open three PRs:

one that update all the lock files for the main CIs. This is expected to pass most of the times (or at least not fail very often)
one for scipy-dev. This may fail a bit more often, most of the time not because of the lock file update, but because of a change in our dev dependencies.
one for CirrusCI arm tests
one for PyPy. I can remove this one since the PyPy build is not in a good state right now.

It adds only a manual trigger (workflow_dispatch) for now but eventually it will be a cron job that updates lock files every one or two weeks.

Another thing that can be done when we are more confident about the bot, is that PRs can be auto-merged: https://github.com/peter-evans/create-pull-request#auto-merge.

This is the kind of PR that is hard to test without it getting merged first.

I did some basic testing in my fork, you can see some PRs created by the bot:
https://github.com/lesteve/scikit-learn/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-desc

I drew some inspiration from Scitools/iris since they have a similar setup #22425 (comment)

github-actions · 2023-10-19T15:41:35Z

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: 2d8d362. Link to the linter CI: here}

glemaitre

I am happy with this PR. It is exactly what I was dreaming of ;)

So now there is 2 things that we can think of:

Having a weekly CRON job for main. IMO, it could straight in a new PR and merge (we can keep it out of this PR if someone wants to try the action first).
Adding an auto-merge of the PR if the CI does not fail. I think that we can wait for some time to see how the manual step is going but we can trigger the option quite soon.

glemaitre · 2023-10-23T11:16:36Z

@lesteve Could solve the conflicts.

Do you think that we could already have the auto-merge but commented with a comment?

…nto update-lock-files-bot

lesteve · 2023-10-23T14:00:13Z

Do you think that we could already have the auto-merge but commented with a comment?

I would leave auto-merge for later for now and do it manually one or two times to make sure everything works as expected.

Adding auto-merge would be something like this:

- name: Enable Pull Request Automerge
      run: gh pr merge --merge --auto "1"
      env:
        GH_TOKEN: ${{ secrets.PAT }}

For this to work we will need to create a bot that has admin privileges so that it can merge PRs. There probably needs to be an auto-approval too, so that required checks are met (we require one approval right now).

ogrisel · 2023-10-23T16:00:13Z

There probably needs to be an auto-approval too, so that required checks are met (we require one approval right now).

I don't think we can auto-approve our own PRs, but it's possible for a maintainer to override the review checks.

ogrisel

This is great. I just have a few questions:

what would be the github account set as author of the automatically opened PR (not the committer). On your fork it's @lesteve. Will this be scikit-learn (the org) on the main repo? I am fine with that but it would be implicit.

Some inline questions and comments below:

ogrisel · 2023-10-23T16:07:06Z

.github/workflows/update-lock-files.yml

+          author: "Lock file bot <noreply@github.com>"
+          delete-branch: true
+          branch: auto-update-lock-files-${{ matrix.name }}
+          title: ":lock: :robot: CI Update lock files for ${{ matrix.name }} CI build(s) :lock: :robot:"


What happens at that step if the diff is empty? For instance where the lock files are already up to date?

does it create an empty commit PR?

does the workflow fail?

Ideally, I would like neither option but a message that says that there are no changes to commit and therefore no pull request has been created.

No PR is created if there are no changes and the workflow does not fail.

The peter-evans/create-pull-request hash some log that you can look at with all the details:

build log existing PR which needs to be updated you see at one point

Pushing pull request branch to 'origin/auto-update-lock-files-scipy-dev'

build log no change needed, no PR created you don't see the Pushing pull request branch step

ogrisel · 2023-10-23T16:08:11Z

build_tools/update_environments_and_lock_files.py

+            each
+            for each in filtered_conda_build_metadata_list
+            if not re.search(skip_build, each["build_name"])
+        ]


filtered_pip_build_metadata_list should be filtered the same way below.

lesteve · 2023-10-24T07:36:56Z

what would be the github account set as author of the automatically opened PR (not the committer).

I have used put the scikit-learn-bot token (the one that automatically opens an issue when the CI fails) so that will be the PR author.

lesteve · 2023-10-24T08:41:57Z

I don't think we can auto-approve our own PRs, but it's possible for a maintainer to override the review checks.

Yeah I thought about that, this needs to be looked into in more details, there is a chance that a pull_request_target workflow with the relevant rights may work for the auto-approval + auto-merge.

lesteve · 2023-10-24T15:28:29Z

So by testing a bit more in my fork, I realised that there were some issues when the PR is created by another user. Right now, I see two main solutions:

give scikit-learn-bot Admin rights i.e. move it to the privileged-bots team. This will push to a branch in the scikit-learn main repo and do a PR from it.
do a PR from scikit-learn-bot/scikit-learn as a normal user using push-to-fork, but then scikit-learn-bot needs to fork scikit-learn/scikit-learn so that scikit-learn-bot/scikit-learn exists, not sure who created this account maybe @thomasjpfan? I have this working in my fork https://github.com/lesteve/scikit-learn/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-desc where PRs are created from test-lesteve/scikit-learn.

There may be other work-arounds of course.

lesteve · 2023-10-27T04:59:06Z

but then scikit-learn-bot needs to fork scikit-learn/scikit-learn so that scikit-learn-bot/scikit-learn exists, not sure who created this account maybe @thomasjpfan?

I found a work-around to create scikit-learn-bot/scikit-learn with the GITHUB_BOT_TOKEN from the CI so I think all is good for this PR to be merged and tested for real!

glemaitre

OK so let's merge and make a live-test.

glemaitre · 2023-10-27T12:07:20Z

I launched the workflow here: https://github.com/scikit-learn/scikit-learn/actions/runs/6666949772

lesteve · 2023-10-27T14:03:22Z

So the workflow does not work because on the push to scikit-learn-bot/scikit-learn it sees there is a new worfklow update-lock-files.yml compared to what is already on the fork. The push is rejected because the token does not have the workflow scope (it is very likely to have only public_repo):

Pushing pull request branch to 'fork/auto-update-lock-files-cirrus-arm'
  /usr/bin/git push --force-with-lease fork auto-update-lock-files-cirrus-arm:refs/heads/auto-update-lock-files-cirrus-arm
  To https://github.com/test-lesteve/scikit-learn
   ! [remote rejected]     auto-update-lock-files-cirrus-arm -> auto-update-lock-files-cirrus-arm (refusing to allow a Personal Access Token to create or update workflow `.github/workflows/test-workflow.yml` without `workflow` scope)

There is more explanation there

The easiest fix is to add the workflow scope to BOT_GITHUB_TOKEN but I think @thomasjpfan is the only one who can do this.

ogrisel · 2023-10-30T14:32:37Z

Rather than granting elevated token privileges to the third-party managed action (peter-evans/create-pull-request in this case), I would rather grant it to a step that uses the GitHub maintained gh CLI: gh pr create in this case.

The gh command should be installed by default in github actions. The only contraint is that it can only be used in workflow that has an actions/checkout step but this is already the case here.

EDIT: or alternatively use the github API manually via our own Python script, similarly to what is done for maint_tools/update_tracking_issue.py.

lesteve · 2023-10-30T14:57:34Z

I am pretty sure irrespective of the method you use, you will still need the workflow scope if the bots pushes into its fork branch and there is a new workflow that the fork has not seen before. This is the case for this error since update-lock-file.yml is seen as a new workflow and the push is rejected.

Updating scikit-learn-bot/scikit-learn to main could make the bot work for a while until a new workflow is added in scikit-learn/scikit-learn main branch.

I don't think this is likely to be a security issue as the workflow scope is granted on the scikit-learn-bot account and does not allow to trigger workflow on the scikit-learn/scikit-learn repo.

thomasjpfan · 2023-10-30T17:37:37Z

The scikit-learn-bot user account does not have any elevated permissions to the scikit-learn repo so I think it's safe to give peter-evans/create-pull-request the workflow scope.

I gave the current token the workflow permission and synced the scikit-learn-bot/scikit-learn fork with upstream.

lesteve · 2023-10-30T18:26:27Z

Nice, thanks a lot!

I ran the workflow manually, looks like the first actions have run fine and some pull requests created 🎉

glemaitre · 2023-10-30T18:30:06Z

Nice. Thanks @thomasjpfan and @lesteve

lesteve added 10 commits October 19, 2023 11:49

Make CI quicker to run

1fffc9c

Add update lock file workflow

87f4fec

workflow dispatch

d471b81

fix

33dcdbb

Use matrix

87bcbad

Add skip-build

0e04fd8

Right arg

eb4796d

Fix

9f3e5a0

cleanup

5b3d4eb

Tweak PR title

816d4ea

github-actions bot added the Build / CI label Oct 19, 2023

lesteve changed the title ~~CI Add~~ CI Add workflow to update lock files Oct 19, 2023

lesteve added 2 commits October 19, 2023 17:45

fix

f434b5e

Make it ready for scikit-learn

3af05b9

lesteve force-pushed the update-lock-files-bot branch from 6b588ae to 3af05b9 Compare October 19, 2023 15:46

lesteve mentioned this pull request Oct 19, 2023

Pin dependencies version in CI with lock files with a bot that auto-updates the lockfiles #22425

Closed

lesteve added 3 commits October 20, 2023 14:18

Separate Cirrus arm tests from the main CI

0c3efee

no space in name since it is reused in the branch name

77c5678

Tweak comment

05587cc

lesteve mentioned this pull request Oct 20, 2023

MAINT unpin cython in CI config #27627

Merged

glemaitre approved these changes Oct 23, 2023

View reviewed changes

lesteve added 2 commits October 23, 2023 15:19

Merge branch 'main' of https://github.com/scikit-learn/scikit-learn i…

2db671c

…nto update-lock-files-bot

Run full doc build

cc9fb6e

ogrisel reviewed Oct 23, 2023

View reviewed changes

skip_build for pip too

b9e10d7

lesteve added 2 commits October 24, 2023 17:19

fix

f25f5cb

Remove labelling which needs special rights

8c90398

lesteve added 2 commits October 27, 2023 06:59

Adjustments

1a218a0

cosmetic tweaks

2d8d362

glemaitre approved these changes Oct 27, 2023

View reviewed changes

glemaitre merged commit 5fc67ae into scikit-learn:main Oct 27, 2023

lesteve deleted the update-lock-files-bot branch October 27, 2023 13:31

RUrlus pushed a commit to RUrlus/scikit-learn that referenced this pull request Oct 30, 2023

CI Add workflow to update lock files (scikit-learn#27622)

d5feb2b

glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Oct 31, 2023

CI Add workflow to update lock files (scikit-learn#27622)

1f42ad1

lesteve mentioned this pull request Nov 6, 2023

CI Add cron schedule for lock-file workflow #27732

Merged

REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023

CI Add workflow to update lock files (scikit-learn#27622)

f0bb442

Uh oh!

Conversation

lesteve commented Oct 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Uh oh!

github-actions bot commented Oct 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

glemaitre left a comment

Choose a reason for hiding this comment

Uh oh!

glemaitre commented Oct 23, 2023

Uh oh!

lesteve commented Oct 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ogrisel commented Oct 23, 2023

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

ogrisel Oct 23, 2023

Choose a reason for hiding this comment

Uh oh!

lesteve Oct 24, 2023

Choose a reason for hiding this comment

Uh oh!

ogrisel Oct 23, 2023

Choose a reason for hiding this comment

Uh oh!

lesteve Oct 24, 2023

Choose a reason for hiding this comment

Uh oh!

lesteve commented Oct 24, 2023

Uh oh!

lesteve commented Oct 24, 2023

Uh oh!

lesteve commented Oct 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lesteve commented Oct 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glemaitre left a comment

Choose a reason for hiding this comment

Uh oh!

glemaitre commented Oct 27, 2023

Uh oh!

lesteve commented Oct 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ogrisel commented Oct 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lesteve commented Oct 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomasjpfan commented Oct 30, 2023

Uh oh!

lesteve commented Oct 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glemaitre commented Oct 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lesteve commented Oct 19, 2023 •

edited

Loading

github-actions bot commented Oct 19, 2023 •

edited

Loading

lesteve commented Oct 23, 2023 •

edited

Loading

lesteve commented Oct 24, 2023 •

edited

Loading

lesteve commented Oct 27, 2023 •

edited

Loading

lesteve commented Oct 27, 2023 •

edited

Loading

ogrisel commented Oct 30, 2023 •

edited

Loading

lesteve commented Oct 30, 2023 •

edited

Loading

lesteve commented Oct 30, 2023 •

edited

Loading