-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Use azpysdk ApiStub in CI #44439
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
base: main
Are you sure you want to change the base?
Use azpysdk ApiStub in CI #44439
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request updates the ApiStub generation process in CI to use the new azpysdk tooling instead of the older tox-based approach. The changes address issues with uv virtual environments that were missing pip, and resolve import errors by running apistub from the staging directory instead of the package directory.
Key changes:
- Modified
get_package_wheel_pathto simplify return type and remove unused output token path logic - Changed
skip_installfrom True to False to ensure the package is installed into the venv - Added pip installation for uv-based virtual environments to ensure compatibility with tools that depend on pip
- Updated CI pipeline to use
dispatch_checks.pyinstead ofdispatch_tox.pyand switched from--toxenvto--checksargument
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| eng/tools/azure-sdk-tools/azpysdk/apistub.py | Simplified wheel path resolution, enabled package installation, added absolute path conversions, and changed working directory for apistub execution to staging directory |
| eng/tools/azure-sdk-tools/azpysdk/Check.py | Added pip installation step for uv virtual environments to ensure compatibility with tools requiring pip |
| eng/pipelines/templates/steps/run_apistub.yml | Updated to use new dispatch_checks.py script, changed toxenv to checks argument, and added environment variables for uv configuration |
| try: | ||
| subprocess.check_call(["uv", "pip", "install", "--python", venv_python, "pip"]) | ||
| except subprocess.CalledProcessError as e: | ||
| logger.error(f"Failed to ensure pip in uv venv: {e}") |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If pip installation fails in the uv venv, the error is logged but execution continues. This could lead to failures later when tools that depend on pip are invoked. Consider either raising the exception or at least returning early to prevent subsequent failures.
| logger.error(f"Failed to ensure pip in uv venv: {e}") | |
| logger.error(f"Failed to ensure pip in uv venv: {e}") | |
| raise RuntimeError("Failed to ensure pip is available in the uv virtual environment.") from e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dislike that we special case this. If we need pip for the apistubgen to work, we should add it to the set of requirements for apistubgen environment. That'll address the issue without special casing here.
I could be missing something of course 😆 , lmk.
scbedd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should make the environment install any necessary packages. Is apistubgen perhaps missing a dependency on pip? Or do we just add pip to the set of requirements for the environment.
#42601
pipelines:
Redoing pipelines after editing pip install