Skip to content
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

Set up automation for updating pytype and pyright versions #11484

Closed
JelleZijlstra opened this issue Feb 27, 2024 · 17 comments · Fixed by #11565 or #11575
Closed

Set up automation for updating pytype and pyright versions #11484

JelleZijlstra opened this issue Feb 27, 2024 · 17 comments · Fixed by #11565 or #11575
Labels
project: infrastructure typeshed build, test, documentation, or distribution related

Comments

@JelleZijlstra
Copy link
Member

Both pytype and pyright release pretty frequently and it's useful for us to make sure we test against the latest version. It would be nice to have a GitHub action that automatically updates them for us so we don't have to remember to make a PR. Example PRs: #11483, #11190.

I suppose other dependencies could also be auto-updated but the need isn't as great.

@AlexWaygood
Copy link
Member

We could probably do this fairly easily with dependabot. We wouldn't need to do it with all our dependencies if we don't want to; but also, dependabot now supports setting dependency groups that are all updated together, so it wouldn't be especially noisy to do it for all our dependencies

@JelleZijlstra
Copy link
Member Author

I think I'd probably want new versions of mypy, pyright, and pytype to trigger an immediate (daily) update PR, and everything else can get updated every month.

@Avasam
Copy link
Collaborator

Avasam commented Feb 28, 2024

I don't know if dependabot can detect our deduplicated pyright version though.
If it's automatically handled, it would be fine to have it duplicated again (and maybe use the python wrapper as a dependency in requirements-tests.txt, but even then I'm not sure if the version given as an argument to pyright-action will be picked up.

Edit: Relevant conversation: jakebailey/pyright-action#9 (comment)

@Mr-Sunglasses
Copy link
Contributor

Hey @JelleZijlstra, I created a script that updates dependencies daily and creates a PR. Let me know if this matches our goal and if we need more improvements.

@JelleZijlstra
Copy link
Member Author

Sounds great, thank you! We'll review your PR soon.

@jakebailey
Copy link
Contributor

jakebailey commented Feb 29, 2024

FWIW a strategy that can be used for pyright is to have a package.json in the repo with an exact version, then pull the value out (kinda like you do with the toml now). That's the stragey that we use on TypeChat's python package: https://github.com/microsoft/TypeChat/blob/main/python/package.json#L12

You don't need to actually install it via that mechanism if you don't want.

(I am tempted to just have the pyright-action read out of a package.json, per the above linked request)

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 11, 2024

We now have four potential solutions prototyped, and I think it's time to decide between them:

  1. Carry on pinning dependencies the way we do now, and use a custom script to auto-update our dependencies: Set up automation for updating pytype and pyright versions. #11491
  2. Move our pyright pin to a package.json file and use dependabot to auto-update our dependencies: Setup dependabot for updating pyright, pytype, flake8-pyi and GitHub Actions #11564
  3. Move our pyright pin to requirements.txt and use dependabot to auto-update our dependencies: Install pyright from PyPI #11575
  4. Carry on pinning dependencies the way we do now, and use renovate to auto-update our dependencies: Setup renovate for updating dependencies #11565

I think all four proposals have their pros and cons. Of the four, I like (4) best, then (3). I like (4) because:

  • The configuration for renovate wasn't as complex as I feared it might be
  • It looks like somebody successfully authorised renovate to run on this repository, which I was worried might be a blocker (chore: Configure Renovate #11568)
  • We can use quarterly schedules for dependencies we don't care about updating that regularly; the longest interval dependabot allows is monthly
  • We can have different schedules for different groups of pip dependencies; dependabot only allows us to set one schedule per ecosystem.

I have some issues with adding pyright to our requirements.txt file (briefly outlined in #11575 (comment) / #11575 (comment)) but I think #11575 is a clear improvement over #11564; it's definitely preferable to put our pyright dependency pin in one of our existing config files rather than a "fake" package.json file, in my opinion.

I like #11491 the least out of our currently prototyped options, as I'd prefer to go with a widely used, well-tested tool rather than a custom script.

@srittau
Copy link
Collaborator

srittau commented Mar 11, 2024

Personally, I like (3) best, followed by (4), if(!) the issues mentioned in #11575 can be ironed out in a non-flaky way. Adding pyright to requirements.txt has the advantage that all type checker versions are together in one file, and that users get a full test environment, just by running pip install -r requirements.txt.

@Avasam
Copy link
Collaborator

Avasam commented Mar 11, 2024

Whilst we're on the discussion, do we want to consider handling dependencies updates shared by requirements-test.txt and .pre-commit-config.yaml together? Sure there's a comment (and a think a custom check?) that makes it so if it's automatically updated in one file, we should remember to update it in the other file as well. But it's be nice if our final solution allows both to be handled together automatically, no manual change needed (except when the workflow fails and manual changes to code is needed of course)

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 11, 2024

Whilst we're on the discussion, do we want to consider handling dependencies updates shared by requirements-test.txt and .pre-commit-config.yaml together?

Renovate can update pre-commit config files, though there's a note in the renovate docs saying that the pre-commit maintainers don't like renovate and prefer you to use the pre-commit auto-update bots. I can update #11565 so renovate updates our pre-commit config as well, if that's something we want. I agree it would be nice to have them updated together, and I can't remember the exact details of the pre-commit/renovate argument

@JelleZijlstra
Copy link
Member Author

There's a lot of things the pre-commit maintainers don't like, though it's probably worth figuring out the exact reason for the recommendation here. If Renovate works, I don't see a reason not to use it for pre-commit too.

@JelleZijlstra
Copy link
Member Author

Seems like this is the context: renovatebot/renovate#11166 (comment). Some disagreement about whether to autoupdate the minimum required pre-commit version.

@srittau
Copy link
Collaborator

srittau commented Mar 11, 2024

Of course being able to update pre-commit as well is a big plus for renovate. In that regard, #11565 and #11575 are not mutually exclusive. The latter would work both with dependabot and renovate, but is a requisite for dependabot.

@Avasam
Copy link
Collaborator

Avasam commented Mar 11, 2024

The fact that we can somewhat easily achieve linked dependencies upgrade across files with Renovate, and that it seems a bit more future-proof and featureful than Dependabot, makes me lean in its favor.

For having used Renovate myself, I especially like all the stats and references that come with PR descriptions.

@srittau
Copy link
Collaborator

srittau commented Mar 12, 2024

So it seems the best option is to go with renovate (#11565) and optionally do #11575 if we get it to work.

@AlexWaygood
Copy link
Member

In that regard, #11565 and #11575 are not mutually exclusive.

That hadn't occured to me, but you're quite right!

So it seems the best option is to go with renovate (#11565) and optionally do #11575 if we get it to work.

That makes sense to me. I'll try to update that PR today so that it updates pre-commit dependencies as well.

@srittau srittau added the project: infrastructure typeshed build, test, documentation, or distribution related label Mar 12, 2024
@AlexWaygood
Copy link
Member

We're now successfully setup with renovate 🎉:

The only thing it looks like it won't do automatically is update our flake8-pyi pin in .pre-commit-config.yaml (because it's listed as an "additional dependency" of the flake8 hook rather than a hook in and of itself)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment