[Docs] Improve the docs and examples #1738
Conversation
Signed-off-by: Jiaxin Shan <[email protected]>
Summary of ChangesHello @Jeffwan, 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 focuses on refining the Aibrix project's documentation and examples to ensure clarity, stability, and correct functionality. It primarily involves updating component image references to the latest stable release, integrating a critical configuration step for Envoy Gateway, and making minor adjustments to quickstart guides and sample configurations for improved user experience and operational correctness. Highlights
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. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on improving documentation and examples, alongside updating image tags to the v0.5.0 release. The changes are beneficial and enhance clarity for users. I've identified a couple of areas for improvement: one regarding the method for configuring envoy-gateway to use a more Helm-native approach for better maintainability, and another to ensure consistency across API endpoint examples in the quickstart guide.
| # patch the configuration to enable EnvoyPatchPolicy, this is super important! | ||
| kubectl apply -f - <<EOF | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: envoy-gateway-config | ||
| namespace: envoy-gateway-system | ||
| data: | ||
| envoy-gateway.yaml: | | ||
| apiVersion: gateway.envoyproxy.io/v1alpha1 | ||
| kind: EnvoyGateway | ||
| provider: | ||
| type: Kubernetes | ||
| gateway: | ||
| controllerName: gateway.envoyproxy.io/gatewayclass-controller | ||
| extensionApis: | ||
| enableEnvoyPatchPolicy: true | ||
| EOF |
There was a problem hiding this comment.
Using kubectl apply to modify a resource managed by Helm can lead to configuration drift and potential issues during chart upgrades, as Helm may not be aware of these manual changes. A more idiomatic and safer approach is to configure envoy-gateway using Helm values during installation.
The envoy-gateway Helm chart likely supports enabling EnvoyPatchPolicy via a value. The installation command could be updated to pass this setting directly. For example:
helm install eg oci://docker.io/envoyproxy/gateway-helm --version v1.2.8 -n envoy-gateway-system --create-namespace \
--set envoyGateway.extensionApis.enableEnvoyPatchPolicy=truePlease verify the exact value path from the envoy-gateway Helm chart's values.yaml and update the documentation to use the --set flag or a values file. This will make the installation process more robust and align with Helm best practices.
There was a problem hiding this comment.
this is not supported yet. envoyproxy/gateway#7458 tracked in this issue
| # list models | ||
| curl -v http://${ENDPOINT}/v1/models | ||
| curl -v http://${ENDPOINT}/v1/models/ |
There was a problem hiding this comment.
You've added a trailing slash to the /v1/models endpoint URL. For consistency, and in case it's needed to avoid redirects, consider adding a trailing slash to the other API endpoint examples in this document as well.
For example:
http://${ENDPOINT}/v1/completionsshould becomehttp://${ENDPOINT}/v1/completions/http://${ENDPOINT}/v1/chat/completionsshould becomehttp://${ENDPOINT}/v1/chat/completions/
Applying this change consistently across all example curl commands would improve the documentation's clarity and correctness.
There was a problem hiding this comment.
/v1/models/ is a little bit different.
1 participant
Pull Request Description
[Please provide a clear and concise description of your changes here]
Related Issues
Resolves: #[Insert issue number(s)]
Important: Before submitting, please complete the description above and review the checklist below.
Contribution Guidelines (Expand for Details)
We appreciate your contribution to aibrix! To ensure a smooth review process and maintain high code quality, please adhere to the following guidelines:
Pull Request Title Format
Your PR title should start with one of these prefixes to indicate the nature of the change:
[Bug]: Corrections to existing functionality[CI]: Changes to build process or CI pipeline[Docs]: Updates or additions to documentation[API]: Modifications to aibrix's API or interface[CLI]: Changes or additions to the Command Line Interface[Misc]: For changes not covered above (use sparingly)Note: For changes spanning multiple categories, use multiple prefixes in order of importance.
Submission Checklist
By submitting this PR, you confirm that you've read these guidelines and your changes align with the project's contribution standards.