Use New VerifyTypes and Pyright Checks in CI #43649
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the Pyright and VerifyTypes CI checks by transitioning from the tox-based approach to a new direct execution method using the run_venv_command abstraction. The changes improve code maintainability by consolidating shared functionality and updating the CI pipeline configuration to use the new dispatch_checks.py script with enhanced environment settings.
Key Changes:
- Refactored
verifytypes.pyto use instance methods andrun_venv_commandabstraction - Updated
pyright.pyto userun_venv_commandwith improved error logging - Modified CI pipeline templates to use new dispatch script with updated environment variables
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
eng/tools/azure-sdk-tools/azpysdk/verifytypes.py |
Converted standalone functions to class methods, integrated run_venv_command abstraction, and added create_package_and_install for package preparation |
eng/tools/azure-sdk-tools/azpysdk/pyright.py |
Replaced direct check_call with run_venv_command and enhanced error logging |
eng/pipelines/templates/steps/run_pyright.yml |
Updated to use new dispatch_checks.py script, simplified arguments, and added environment variable configuration |
| return 0 | ||
|
|
||
| def get_type_complete_score( | ||
| self, executable, commands: typing.List[str], cwd: str, check_pytyped: bool = False |
There was a problem hiding this comment.
The parameter name executable is ambiguous. Consider renaming it to python_executable to match the parameter name used in the install_from_main method and improve clarity about what executable is being referenced.
| return 0 | ||
|
|
||
| def get_type_complete_score( | ||
| self, executable, commands: typing.List[str], cwd: str, check_pytyped: bool = False |
There was a problem hiding this comment.
This method lacks a docstring. Add a docstring that describes the purpose of the method, explains the parameters (especially executable, commands, cwd, and check_pytyped), documents the return value (the type completeness score), and explains when -1.0 is returned to indicate an error.
| results.append(1) | ||
| return max(results) if results else 0 | ||
|
|
||
| def install_from_main(self, setup_path: str, python_executable: Optional[str] = None) -> int: |
There was a problem hiding this comment.
This method lacks a docstring. Add a docstring that describes the method's purpose (installing a package from the main branch), explains the parameters (setup_path and python_executable), documents the return value (0 for success, 1 for failure), and describes the cleanup behavior with the temporary directory.
1 participant
#42883
run_venv_commandabstraction in both scriptspipeline run examples
(verifytypes only, pyright skipped)
(with pyright)
intentional pyright & verifytypes error from this commit:
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=5503339&view=logs&j=b70e5e73-bbb6-5567-0939-8415943fadb9&t=56d4e2e6-c43b-527c-bad7-234ebe7429dd