[diffusion] Auto-skip diffusion tests when required pipeline class is missing from diffusers #21139
Conversation
|
/tag-and-rerun-ci |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the stability and robustness of the CI pipeline for diffusion-related tests. It introduces error handling that intelligently skips tests when the installed Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a helpful change to automatically skip diffusion tests when a required pipeline class is missing from the installed diffusers version. This improves CI stability by preventing unrelated jobs from being cancelled. The implementation correctly uses a try...except block to catch server startup errors. My review includes a suggestion to make the error message checking more specific to diffusers to avoid unintentionally skipping tests due to unrelated errors, thus improving the robustness of the tests.
| # pipeline class. This avoids hard failures when a model needs a | ||
| # newer diffusers release than what is currently installed in CI. | ||
| msg = str(exc) | ||
| if "not found in diffusers" in msg or "has no attribute" in msg: |
There was a problem hiding this comment.
The check for "has no attribute" in msg is too broad and could unintentionally skip tests that fail due to unrelated AttributeErrors, masking potential bugs. To make the check more robust, it should be scoped to errors originating from the diffusers library. This suggestion also adds a check for ImportError, which can also occur with older diffusers versions when a class is not found.
if ("not found in diffusers" in msg
or ("cannot import name" in msg and "from 'diffusers" in msg)
or ("has no attribute" in msg and "diffusers" in msg)):|
/tag-and-rerun-ci |
…s is missing from diffusers (sgl-project#21139)
…s is missing from diffusers (sgl-project#21139)
…s is missing from diffusers (sgl-project#21139)
Motivation
When a diffusion test case requires a pipeline class not available in the installed diffusers version, automatically mark it as skipped instead of failing, which prevents fail-fast from cancelling unrelated jobs in the CI matrix.
Modifications
Accuracy Tests
Benchmarking and Profiling
Checklist
Review Process
/tag-run-ci-label,/rerun-failed-ci,/tag-and-rerun-ci