Skip to content

Setup dependabot for updating pyright, pytype, flake8-pyi and GitHub Actions #11564

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

Closed
wants to merge 4 commits into from

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Mar 10, 2024

Dependabot will do a daily check to see if there is a new version of pyright, pytype or flake8-pyi, and will create PRs updating them if so. It will also do a monthly check to see if there are any new versions for any of our GitHub actions, and will create a PR updating all of them together if so.

Dependabot will not check to see if any of our other dependencies are out of date. Ideally (in my opinion) it would do a monthly or quarterly check for those. Unfortunately, it doesn't look like it's possible to have two entries with different schedules for a single package-ecosystem.

In order to use dependabot with our pyright dependency, we switch to storing the pyright dependency in a minimal package.json file rather than as a field in our pyproject.toml file. This is the solution suggested by @jakebailey in #11491 (comment).

Fixes #11484
Closes #11491
Closes #11565

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Mar 10, 2024

Ugh, seems like this breaks the stub-uploader, which depends on the tool.typeshed.pyright_version field of our pyproject.toml...

https://github.com/python/typeshed/actions/runs/8223349890/job/22485946355?pr=11564

Comment on lines +2 to +3
"name": "typeshed",
"version": "0.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

These fields are required in order for this to be considered a valid package.json file: https://docs.npmjs.com/creating-a-package-json-file#packagejson-fields

Copy link
Collaborator

@Avasam Avasam Mar 10, 2024

Choose a reason for hiding this comment

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

Just FYI if you ever want to add a comment to a package.json file, just add any top level field that starts with //. ie:
"//": "my comment", "// some-packge": "other comment about a package, with a unique comment key". The "official" (as official as a mailing list can be) recommendation is to make "//" an array, but that lacks comment positioning, so I've been doing it the way I described for years.
https://groups.google.com/g/nodejs/c/NmL7jdeuw0M/m/yTqI05DRQrIJ

@AlexWaygood AlexWaygood marked this pull request as ready for review March 10, 2024 16:39
@AlexWaygood
Copy link
Member Author

Ugh, seems like this breaks the stub-uploader, which depends on the tool.typeshed.pyright_version field of our pyproject.toml...

python/typeshed/actions/runs/8223349890/job/22485946355?pr=11564

I fixed this failure by keeping the redundant tool.typeshed.pyright_version field in pyproject.toml for now, even though none of our tooling will actually use that field if this PR is merged. Once we've merged this PR, we can update the stub-uploader to read from the package.json file instead, and then we can remove the redundant field from our pyproject.toml file here.

@srittau
Copy link
Collaborator

srittau commented Mar 10, 2024

I'm not super happy with this solution. I'd prefer to keep all meta data in one file, instead of spreading it over multiple files. I also don't think we should add a "fake" package.json file just to make dependabot happy. While it's more work, I'd prefer to use a custom solution for pyright or just keep updating it manually.

@AlexWaygood
Copy link
Member Author

I'd prefer to keep all meta data in one file, instead of spreading it over multiple files.

That's definitely my preference too! But I'd also prefer to use widely used, well-tested tools for updating dependencies where possible. In this case, I personally weigh that more heavily than my preference for keeping metadata consolidated.

But I agree this isn't ideal.

@@ -0,0 +1,29 @@
version: 2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just so you know, dependabot will automatically add the dependencies label with default configs (which I believe is something we'd want, but has only been used for a single PR on typeshed before)
https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#labels

Copy link
Collaborator

@Avasam Avasam left a comment

Choose a reason for hiding this comment

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

Looks good to me. I double checked, Dependabot seems to bee looking for any file that contains the string requirements in the name or any .txt file that passes their requirements parsing test, so it should pickup ours.
https://github.com/dependabot/dependabot-core/blob/main/python/lib/dependabot/python/file_fetcher.rb

As a bonus, we could get security advisory warnings about dependencies. Although typeshed stays pretty up to date with minimal deps so we're probably gonna outpace CVE publications.
https://docs.github.com/en/code-security/dependabot/dependabot-security-updates/configuring-dependabot-security-updates

Copy link
Collaborator

@Avasam Avasam left a comment

Choose a reason for hiding this comment

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

Edit: I've been missing on conversations as my page was opened.

I'm not super happy with this solution. I'd prefer to keep all meta data in one file, instead of spreading it over multiple files. I also don't think we should add a "fake" package.json file just to make dependabot happy. While it's more work, I'd prefer to use a custom solution for pyright or just keep updating it manually.

Thoughts on using https://pypi.org/project/pyright/ ? We could then also simplify tests/pyright_test.py and scripts/runtests.py since we wouldn't need to manually check for node install anymore. Nor have to parse the pyright version.

I fixed this failure by keeping the redundant tool.typeshed.pyright_version field in pyproject.toml for now, even though none of our tooling will actually use that field if this PR is merged. Once we've merged this PR, we can update the stub-uploader to read from the package.json file instead, and then we can remove the redundant field from our pyproject.toml file here.

^ explains why the pyproject.toml is left in for now.

Still rescinded my review for now to further discuss @srittau 's concern. But I tend to agree with Alex here if we can't find anything better.

@Avasam Avasam self-requested a review March 10, 2024 17:49
@AlexWaygood
Copy link
Member Author

I'm currently playing around with an alternative solution that uses renovate.

@jakebailey
Copy link
Contributor

jakebailey commented Mar 10, 2024

Yeah, if you're willing to use renovate, the regex manager can bump anything anywhere: https://docs.renovatebot.com/modules/manager/regex/

[tool.typeshed]
# renovate: datasource=npm depName=pyright
pyright_version = "1.1.350"

renovate ships with some presets, but you can make your own based on them, e.g. the GitHub Actions workflow scanner that looks for variables named *_VERSION: https://docs.renovatebot.com/presets-regexManagers/#regexmanagersgithubactionsversions

(would have to modify a complicated regex, or just hardcode one manager for this purpose)

@AlexWaygood
Copy link
Member Author

See #11565 for what a solution using renovate might look like

@srittau
Copy link
Collaborator

srittau commented Mar 10, 2024

Thoughts on using https://pypi.org/project/pyright/ ?

This sounds like the best solution to me. Are there potential problems with it? Otherwise, by just adding it to requirements, we'd also provide a neat way for testing locally.

If this would be problematic, using renovate also sounds promising.

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.

Set up automation for updating pytype and pyright versions
4 participants