[Misc] Add Makefile to run full pipeline with a single command #675
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @ikaadil, 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 significantly enhances the developer workflow by introducing a Makefile that consolidates and automates the execution of the entire project pipeline. This change provides a single, consistent entry point for running all necessary tests and checks, making it easier for developers to set up, validate, and contribute to the project.
Highlights
- New Makefile Introduced: A new
Makefilehas been added to the repository to centralize and simplify the execution of various development and testing pipeline steps. - Unified Pipeline Execution: The
Makefileincludes analltarget that allows running a comprehensive set of tests and checks with a single command, covering Python tests, pre-commit hooks, router E2E tests, Helm chart functionality tests, and operator CRD tests. - Improved Developer Experience: This addition aims to enhance developer productivity and onboarding by providing a standardized and simplified workflow for running the project's pipeline, reducing the need to remember or manually chain multiple commands.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This PR introduces a comprehensive Makefile to automate various testing and linting pipelines, which is a great step towards improving developer productivity. The Makefile is well-structured and covers a wide range of scenarios. My review includes suggestions to enhance maintainability by reducing code duplication, improve the reliability of tests by replacing fixed sleeps with active polling, and follow best practices for command chaining and dependency management.
…rs (vllm-project#654) * Add --k8s-timeout-seconds argument for Kubernetes watcher streams Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Enhance Kubernetes service discovery by adding configurable timeout for watcher streams - Introduced parameter in and classes to allow dynamic timeout settings. - Updated relevant methods to utilize the new timeout configuration instead of hard-coded values. Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Validate k8s-timeout-seconds argument to ensure it is greater than 0 in the parser. This change enhances error handling for Kubernetes service discovery configurations. Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Refactor Kubernetes service discovery timeout parameters to use 'watcher_timeout_seconds' instead of 'timeout_seconds'. Updated related validation and argument parsing to reflect this change, enhancing clarity and configurability for watcher stream timeouts. Signed-off-by: Ifta Khairul Alam Adil <[email protected]> --------- Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
Signed-off-by: Rui Zhang <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
Signed-off-by: Nicolas Jourdan <[email protected]> Co-authored-by: Nicolas Jourdan <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
* add doc/exemple Signed-off-by: Redno2 <[email protected]> * add endFrom secret Signed-off-by: Redno2 <[email protected]> * fix add endFrom secret Signed-off-by: Redno2 <[email protected]> * missing config for initContainer Signed-off-by: Redno2 <[email protected]> --------- Signed-off-by: Redno2 <[email protected]> Co-authored-by: Yuhan Liu <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
* [CI] remove docker image before building router image Signed-off-by: Rui Zhang <[email protected]> * [Bugfix] Fix routing to deleted endpoint Signed-off-by: Rui Zhang <[email protected]> * Revert "[CI] remove docker image before building router image" This reverts commit 8319e01. Signed-off-by: Rui Zhang <[email protected]> * make health check timeout configurable Signed-off-by: Rui Zhang <[email protected]> --------- Signed-off-by: Rui Zhang <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
…es (vllm-project#647) Signed-off-by: Mahmoud Frouk <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
…hecks, and various E2E functionality tests - Introduced a comprehensive Makefile to streamline CI/CD operations. - Added targets for running Python tests, pre-commit checks, and multiple E2E tests (Router, Kubernetes discovery, Static discovery). - Included functionality tests for secure, two-pod, and multiple model deployments. - Integrated operator CRD testing to validate custom resource definitions. This structure enhances automation and simplifies the testing process across different environments. Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
- Simplified Python tests execution by removing conditional checks and directly using . - Updated pre-commit checks to utilize for linting instead of the previous method. - Removed redundant checks for pre-commit installation in manual linting tasks, directly invoking pre-commit commands. - Enhanced Router E2E tests by streamlining dependency installations and test executions. This refactor improves clarity and efficiency in the CI/CD workflow. Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
- Added port availability check to ensure port 8000 is free before running tests. - Upgraded pip before installing dependencies for improved reliability. - Streamlined the execution of performance tests and logging for better clarity and debugging. - Increased the duration of the request generator to 300 seconds for more extensive testing. These changes improve the robustness and clarity of the Router E2E testing process. Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
- Enhanced Kubernetes discovery and static discovery E2E test scripts for better error handling and clarity. - Added checks for required CLI tools and improved dependency installation processes. - Streamlined Docker image building and Helm deployment steps with clearer logging and cleanup procedures. - Implemented environment setup for conda activation and ensured proper cleanup of resources after tests. These changes enhance the robustness and usability of the E2E testing framework. Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
…lm-project#469) * [feat]: add transcription API endpoint using OpenAI Whisper-small Signed-off-by: David Gao <[email protected]> * remove the whisper payload response log Signed-off-by: David Gao <[email protected]> * [docs]: add tutorial for transcription v1 api Signed-off-by: David Gao <[email protected]> * [chore] align example router running script with main new script will be mentioned in `tutorials/17-whisper-api-transcription.md` Signed-off-by: David Gao <[email protected]> * omit model field since backend already knows which model to run Signed-off-by: David Gao <[email protected]> * generate a silent audio file if no audio file appears Signed-off-by: David Gao <[email protected]> * put wav creation at the module level to prevent being recreated every time Signed-off-by: David Gao <[email protected]> * [Test] test frequency of silent audio creation Signed-off-by: David Gao <[email protected]> * send multipart/form-data for transcription model's health check Signed-off-by: David Gao <[email protected]> * fix pre-commit issue Signed-off-by: David Gao <[email protected]> * Moves the implementation for the `/v1/audio/transcriptions` endpoint from `main_router.py` into `request.py`, align architectural pattern. Signed-off-by: David Gao <[email protected]> * add timeout to ensure health check will not hang indefinitely if a backend model becomes unresponsive Signed-off-by: David Gao <[email protected]> * add boolean model health check return for non-transcription model Signed-off-by: David Gao <[email protected]> * remove redundant warning log since handled in outer 'StaticServiceDiscovery.get_unhealthy_endpoint_hashes' Signed-off-by: David Gao <[email protected]> * remove redundant JSONDecodeError catch and downgrade RequestException log to debug, align with service discovery's warning Signed-off-by: David Gao <[email protected]> * Chore: Apply auto-formatting and linting fixes via pre-commit Signed-off-by: David Gao <[email protected]> * refactor: update more meaningful comments for silent wav bytes generation Signed-off-by: David Gao <[email protected]> * refactor: keep the comment to explain purpose for generating a silent WAV byte Signed-off-by: David Gao <[email protected]> * fix(tests): Improve mock in model health check test The mock for `requests.post` in `test_is_model_healthy` did not correctly simulate an `HTTPError` on a non-200 response. This change configures the mock's `raise_for_status` method to raise the appropriate exception, ensuring the test now accurately validates the function's error handling logic. Signed-off-by: David Gao <[email protected]> * Chore: Apply auto-formatting and linting fixes via pre-commit Signed-off-by: David Gao <[email protected]> * chore: remove unused var `in_router_time` Signed-off-by: David Gao <[email protected]> * fix: (deps) add httpx as an explicit dependency The CI/CD workflow was failing with a `ModuleNotFoundError` because `httpx` was not an explicit dependency in `pyproject.toml` and was not being installed in the clean Docker environment. Signed-off-by: David Gao <[email protected]> * chore: dependencies order changes after running pre-commit Signed-off-by: David Gao <[email protected]> * refactor: Migration from httpx to aiohttp for improved concurrency Replaced httpx with aiohttp for better asynchronous performance and resource utilization. Fixed JSON syntax error in error response handling. Signed-off-by: David Gao <[email protected]> * chore: remove wrong tutorial file Signed-off-by: David Gao <[email protected]> * chore: apply pre-commit Signed-off-by: David Gao <[email protected]> * chore: use debug log print Signed-off-by: David Gao <[email protected]> * chore: change to more specific exception handling for aiohttp Signed-off-by: David Gao <[email protected]> --------- Signed-off-by: David Gao <[email protected]> Co-authored-by: Yuhan Liu <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
- Simplified Python tests execution by removing conditional checks and directly using . - Updated pre-commit checks to utilize for linting instead of the previous method. - Removed redundant checks for pre-commit installation in manual linting tasks, directly invoking pre-commit commands. - Enhanced Router E2E tests by streamlining dependency installations and test executions. This refactor improves clarity and efficiency in the CI/CD workflow. Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
ikaadil
force-pushed
the
ci-cd/make_file
branch
from
b6ee1f1 to
9576c5b
Compare
…d compliance with pre-commit checks. Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
|
@zerofishnoodles Can you review this PR? Thanks |
Thanks for the feedback! 🙏 The main reason I added the I’m open to moving it under |
7 participants
Summary
This PR introduces a Makefile that simplifies running the entire pipeline with a single command.
It aims to improve developer experience by reducing the need to remember or manually chain multiple commands.
Changes
Makefilewith targets to run the full pipelineBenefits
Notes