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

Support for bumping pyright version using dependabot #9

Closed
jenshnielsen opened this issue Jan 30, 2023 · 15 comments
Closed

Support for bumping pyright version using dependabot #9

jenshnielsen opened this issue Jan 30, 2023 · 15 comments

Comments

@jenshnielsen
Copy link

Thanks for a great action,

It would be nice to be able to pin the pyright version used but have in automatically upgraded. Would it be possible to specify the version of pyright in a package.json file or similar such that dependabot or similar tool automatically can update the version when a new pyrigth version is released. Could this be possible?

@jakebailey
Copy link
Owner

That's an interesting idea, though I don't quite know I feel about Python projects sticking package.json and package-lock.json into their repos and then not using them for anything but pyright.

It doesn't look like dependabot is adding any new ecosystems either, so I don't have the opportunity to encode this elsewhere.

Maybe it'd be possible to pin via the pyright PyPI package (and detect that it's already installed in PATH), if that's something you might find more reasonable.

@jenshnielsen
Copy link
Author

@jakebailey I guess that could work. I have been trying to avoid using the pip version to not pull in an extra node environment within my pip install.

I don't quite know I feel about Python projects sticking package.json and package-lock.json into their repos and then not using them for anything but pyright.

Agree that's not the greatest. It would be better if the files could have a less generic name. As a partial mitigation I guess its possible to have the file in another location than the root to make this less confusing such as .github/pyright/package.json ?
This should work since dependabot can be configured to look in multipel directories

@jakebailey
Copy link
Owner

Sticking a package.json into .github could work. One immediate trouble I can imagine is that the version specified in the option currently is an exact version (or latest), but package.json would be semver, so I'd need to decide what to do about that, as well as what do about lock files.

@jenshnielsen
Copy link
Author

Yes, I would expect the user want to specify the full version since it in my experience is fairly common for typechecking to change between 2 patch versions causing new errors

@jakebailey
Copy link
Owner

jakebailey commented Jul 10, 2023

I've been playing around with renovate; one thing it supports is a way to annotate parts of files to indicate that they are in fact package versions. I think one can do this:

env:
  # renovate: datasource=npm depName=pyright
  PYRIGHT_VERSION: '1.1.311'

- uses: jakebailey/pyright-action@v1
  with:
    version: ${{ env.PYRIGHT_VERSION }}

Then add the regexManagers:githubActionsVersions preset to a renovate config.

@jakebailey
Copy link
Owner

jakebailey commented Jul 10, 2023

That preset only works for vars that end in _VERSION, but here's a working example:

If one opts for copying the preset and modifying it to work on the version: line, that would work too, but an env var is okay.

@Avasam
Copy link

Avasam commented Feb 29, 2024

It doesn't look like dependabot is adding any new ecosystems either, so I don't have the opportunity to encode this elsewhere.

Looks like this is now Contributing new ecosystems

@jakebailey
Copy link
Owner

Yeah, though I'm not totally sure what it'd look like 😄

I'm tempted to just add support for reading the version out of a package.json, as originally suggested. That's what we do in TypeChat: https://github.com/microsoft/TypeChat/blob/main/python/package.json#L12

@ewjoachim
Copy link

ewjoachim commented Mar 14, 2024

So, just wanted to weight in: I'm using this action, and also the pre-commit hook from https://github.com/RobertCraigie/pyright-python (the python package wrapper over the pyright package, that is listed in the pyright doc as the way to go.

Do you think there could be a way to ensure that the pyright version used by both system is the same AND is updated by renovate/dependabot ? Given the way pre-commit works, I'm enclined to say the only way I see would be to extract the version from pre-commit with:

$ yq '.repos | filter(.repo == "https://github.com/RobertCraigie/pyright-python").0.rev | "pyright-version="+sub("^v", "")' >> $GITHUB_OUTPUT

(yq is pre-installed on GHA)

So I'd need a step for that, and then use

- uses: jakebailey/pyright-action@v1
  with:
    version: ${{ steps.<id>.outputs.pyright-version }}

Is there a better way or should this be the way ? If so, should it be documented as such ?

@jakebailey
Copy link
Owner

jakebailey commented Mar 14, 2024

As more projects keep pinning pyright using different mechanisms (package.json, requirements.txt, pyproject, precommit), I'm less included to try and include something here just becuase it sounds like a slippery slope of every format that could exist.

I have no problem with providing examples in the README using the different methods, though.

@jenshnielsen
Copy link
Author

@jakebailey Given that you now support using an installed version from PATH (and thanks for that) I think we can safely close this issue. The user is free to manage installation of pyright in a way that they see fit, which can then be updated using dependabot or a similar tool

@jakebailey
Copy link
Owner

Yes, I agree!

There is still #98, where parsing a version and passing it in may still be valuable, but I think the verison: PATH method is not too bad if you're willing to pull in the PyPI package.

@jenshnielsen
Copy link
Author

@jakebailey FYI installing the version via NPM should also work fine. Here is an example that I hacked together of doing that microsoft/Qcodes#5958

@jakebailey
Copy link
Owner

Indeed; though I largely think parsing it is going to be faster, slightly. I have this in TypeChat: https://github.com/microsoft/TypeChat/blob/f53b971179d0136424a75d67287903a2421af98b/.github/workflows/ci.python.yml#L52

@jenshnielsen
Copy link
Author

@jakebailey Thanks for the tip. That is indeed a lot simpler and nicer

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

No branches or pull requests

4 participants