chore(pyright): rebuild pyright pins on each buildkite build #20440
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @rexledesma and the rest of your teammates on |
rexledesma
force-pushed
the
rl/update-pins-pyright-build
branch
2 times, most recently
from
0f3dda9 to
5c2a6d8
Compare
smackesey
reviewed
Mar 13, 2024
There was a problem hiding this comment.
Thanks for moving this forward. A few points:
make rebuild_pyright_pinswill run pyright after building pins on its own, so there is no need for a follow-upmake pyright- One reason the pins system is in place is to keep
pyrightresults the same between local dev and CI. That would no longer be true after this PR-- a new dep could come out that changes annotations and causes irritating build errors ("unnecessary type ignore" highly likely from fixed annotations) in CI.
Bc of above I think we need:
- A separate step that runs only on
requirements.txtchange (would be nice if it stays in the same BK UI group) - The
run-pyrightscript needs to be modified with an option that will skip running pyright itself after building the venv, since we only care that the env builds, not that there are no type errors under the new pins.I am going to spin out a quick PR for this now that you can stack on.
PRs that add --skip-typecheck:
|
#20565 has landed, so reopening this. |
rexledesma
force-pushed
the
rl/update-pins-pyright-build
branch
from
5c2a6d8 to
ee27209
Compare
rexledesma
force-pushed
the
rl/update-pins-pyright-build
branch
2 times, most recently
from
032addc to
2a11b0e
Compare
smackesey
reviewed
Mar 19, 2024
rexledesma
force-pushed
the
rl/update-pins-pyright-build
branch
from
2a11b0e to
d4b9f6d
Compare
smackesey
approved these changes
Mar 20, 2024
PedramNavid
pushed a commit
that referenced
this pull request
Mar 28, 2024
## Summary & Motivation Prevent errors like #20406 from happening by always attempting to regenerate the pyright pins on each build. I could put this in a separate step and scope it down to only run if any `requirements.txt` has changed in the `pyright/` subdirectory, but want to see if the build time difference is negligible because of `uv` first. ## How I Tested These Changes bk
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary & Motivation
Prevent errors like #20406 from happening by always attempting to regenerate the pyright pins on each build.
I could put this in a separate step and scope it down to only run if any
requirements.txthas changed in thepyright/subdirectory, but want to see if the build time difference is negligible because ofuvfirst.How I Tested These Changes
bk