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

Use 'version: PATH' for pyright-action #11743

Merged
merged 4 commits into from
Apr 12, 2024
Merged

Conversation

jakebailey
Copy link
Contributor

I've just updated pyright-action to be able to pull pyright from $PATH. This eliminates the need to parse out a pyright version, download, etc, of the environment already has it installed, which is the case for the workflows in this repo.

https://github.com/jakebailey/pyright-action#using-pyright-from-path

@jakebailey
Copy link
Contributor Author

Uh, huh. I guess pyright --version returns something strange in this repo.

@jakebailey jakebailey marked this pull request as draft April 11, 2024 05:19
@jakebailey
Copy link
Contributor Author

added 1 package, and audited 2 packages in 2s

found 0 vulnerabilities
pyright 1.1.357
WARNING: there is a new pyright version available (v1.1.357 -> v1.1.358).
Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest`

😐

@Akuli
Copy link
Collaborator

Akuli commented Apr 11, 2024

We are using https://pypi.org/project/pyright/ which apparently installs pyright when it's invoked for the first time.

If you don't want to parse that output in the action (that is, ignore all lines that don't look like pyright <version>), you could invoke pyright --version twice and look at only the second output, or at least that works for me locally:

(e) akuli@akuli-desktop:~$ pyright --version
/tmp/e/lib/python3.11/site-packages/nodeenv.py:26: DeprecationWarning: 'pipes' is deprecated and slated for removal in Python 3.13
  import pipes
 * Install prebuilt node (21.7.3) ..... done.

added 1 package, and audited 2 packages in 3s

found 0 vulnerabilities
npm notice 
npm notice New patch version of npm available! 10.5.0 -> 10.5.2
npm notice Changelog: https://github.com/npm/cli/releases/tag/v10.5.2
npm notice Run npm install -g npm@10.5.2 to update!
npm notice 
pyright 1.1.358

(e) akuli@akuli-desktop:~$ pyright --version
pyright 1.1.358

@jakebailey
Copy link
Contributor Author

jakebailey commented Apr 11, 2024

If you don't want to parse that output in the action (that is, ignore all lines that don't look like pyright <version>), you could invoke pyright --version twice and look at only the second output, or at least that works for me locally:

Yeah, running it twice was what I was thinking of doing, but it sure seems like a bit of a waste now that I'm seeing that the pyright PyPI package doesn't actually bundle pyright 😞

The parsing / pyright-action downloading will be faster than that...

@jakebailey
Copy link
Contributor Author

Honestly, I'm tempted to file a bug on the pyright PyPI package; this all should be piped to stderr. That or I did something wrong and am accidentally parsing stderr!

@Akuli
Copy link
Collaborator

Akuli commented Apr 11, 2024

It is outputting some garbage to stdout, and it's quite inconsistent about it:

(e) akuli@akuli-desktop:/tmp$ rm -rf ~/.cache/pyright-python/
(e) akuli@akuli-desktop:/tmp$ pyright --version >out 2>err
(e) akuli@akuli-desktop:/tmp$ cat out

added 1 package, and audited 2 packages in 3s

found 0 vulnerabilities
pyright 1.1.358
(e) akuli@akuli-desktop:/tmp$ cat err
/tmp/e/lib/python3.11/site-packages/nodeenv.py:26: DeprecationWarning: 'pipes' is deprecated and slated for removal in Python 3.13
  import pipes
 * Install prebuilt node (21.7.3) ..... done.

@Akuli
Copy link
Collaborator

Akuli commented Apr 11, 2024

Apparently it doesn't output any garbage to stdout if you pass --outputjson, which is special-cased in pyright-python, but pyright ignores it when you also pass --version.

(e) akuli@akuli-desktop:/tmp$ rm -rf ~/.cache/pyright-python/
(e) akuli@akuli-desktop:/tmp$ pyright --version --outputjson >out 2>err
(e) akuli@akuli-desktop:/tmp$ cat out
pyright 1.1.358

Now that I've looked at pyright-python's source code, I'll make a PR there :)

@Akuli Akuli added the status: deferred Issue or PR deferred until some precondition is fixed label Apr 11, 2024
@jakebailey
Copy link
Contributor Author

jakebailey commented Apr 12, 2024

I've updated the action to be a little more permissive when parsing versions, so it should be able to pull out the version even in mixed-output situations, and will retry the command if needed too.

Of course, having pyright-python instead put this stuff on stderr would be a good idea in general.

@jakebailey jakebailey marked this pull request as ready for review April 12, 2024 17:13
@Akuli Akuli removed the status: deferred Issue or PR deferred until some precondition is fixed label Apr 12, 2024
Copy link
Collaborator

@Akuli Akuli left a comment

Choose a reason for hiding this comment

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

Thanks! Now that the feature exists, there's IMO no reason for us to parse the version like we did before.

@Akuli Akuli merged commit b61d90c into python:main Apr 12, 2024
41 checks passed
@jakebailey jakebailey deleted the pyright-path branch April 12, 2024 17:20
@AlexWaygood
Copy link
Member

Thanks @jakebailey!

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.

3 participants