Maya Fast Api deployment on Modal#3
Conversation
WalkthroughA new Modal-based ASGI application is introduced in Changes
Sequence DiagramsequenceDiagram
participant Modal Runtime
participant modal_app.py
participant .env File
participant maya1.api_v2
Modal Runtime->>modal_app.py: Invoke maya1_tts_app()
modal_app.py->>+.env File: Load environment variables
.env File-->>-modal_app.py: Variables loaded
modal_app.py->>modal_app.py: Check MAYA1_MODEL_PATH
Note over modal_app.py: If not set, default to<br/>"maya-research/maya1"
modal_app.py->>+maya1.api_v2: Import FastAPI app
maya1.api_v2-->>-modal_app.py: Return app instance
modal_app.py-->>Modal Runtime: Return ASGI app
Note over Modal Runtime: App ready for requests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
maya_modal.py (4)
4-4: Consider a more descriptive app name.The app name "sub1" is generic and doesn't reflect the purpose of this deployment. A more descriptive name like "maya1-tts-fastapi" would make it easier to identify in the Modal dashboard.
-app = modal.App("sub1") +app = modal.App("maya1-tts-fastapi")
9-33: Consider pinning dependency versions more strictly.Using
>=for all dependencies allows any future version to be installed, which can lead to unexpected breaking changes or dependency conflicts. For production deployments, consider using more specific version constraints (e.g.,~=for compatible releases or exact versions with==).For example:
- "torch>=2.5.0", - "transformers>=4.57.0", + "torch~=2.5.0", + "transformers~=4.57.0",
46-47: Review container scaling configuration for cost and performance implications.The current configuration sets
min_containers=1andmax_containers=1, which means:
- Cost: The container will run continuously 24/7, even with no traffic, incurring ongoing GPU costs.
- Performance: The service cannot scale horizontally under load, potentially causing request queuing or timeouts during high traffic.
Consider whether these constraints are intentional for your use case:
- If you need instant response times without cold starts, keep
min_containers=1.- If you expect variable load, increase
max_containersto allow scaling (e.g.,max_containers=5).- If cost optimization is important and cold starts are acceptable, reduce
min_containers=0.
50-71: Add error handling for imports and initialization.The function lacks error handling around the import of
maya1.api_v2.app. If the import fails (e.g., due to missing dependencies or initialization errors in the FastAPI app), the error won't be caught, making debugging difficult.Consider adding basic error handling:
def maya1_tts_app(): """ Runs inside the Modal container. - Loads env (.env) if present - Sets default model path - Imports FastAPI `app` from maya1.api_v2 - Returns it as the ASGI app """ import os - from dotenv import load_dotenv - - load_dotenv() - - # If not overridden, use HF repo as model path os.environ.setdefault("MAYA1_MODEL_PATH", "maya-research/maya1") - - # If the model is gated and you need auth, you can also set: - # os.environ.setdefault("HUGGINGFACE_HUB_TOKEN", "<your_hf_token>") - - from maya1.api_v2 import app as fastapi_app + + try: + from maya1.api_v2 import app as fastapi_app + except Exception as e: + print(f"Failed to import FastAPI app: {e}") + raise + return fastapi_app
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
maya1_test.wavis excluded by!**/*.wav
📒 Files selected for processing (1)
maya_modal.py(1 hunks)
🔇 Additional comments (1)
maya_modal.py (1)
9-33: Do not add flashinfer-python—it's incompatible with the specified PyTorch version.The requirements.txt file does list
flashinfer-python, but it explicitly documents: "Currently incompatible with PyTorch 2.5+". Since maya_modal.py specifiestorch>=2.5.0, the intentional exclusion of flashinfer-python is correct. The codebase contains no flashinfer imports, confirming it is unused. Adding this dependency would introduce a version conflict.Likely an incorrect or invalid review comment.
| import os | ||
| from dotenv import load_dotenv | ||
|
|
||
| load_dotenv() |
There was a problem hiding this comment.
Critical: .env file won't be available in Modal containers.
The load_dotenv() call will fail silently because .env files are not automatically copied to Modal containers. Environment variables configured via load_dotenv() will not be set, potentially causing the application to fail or use incorrect defaults.
Use Modal Secrets to manage environment variables instead.
Apply this approach:
- Create Modal secrets for your environment variables:
# Set secrets via Modal CLI
modal secret create maya1-secrets \
MAYA1_MODEL_PATH="maya-research/maya1" \
HUGGINGFACE_HUB_TOKEN="your_token_here"- Update the function decorator to use the secrets:
@app.function(
image=image,
gpu="A10G",
timeout=600,
min_containers=1,
max_containers=1,
+ secrets=[modal.Secret.from_name("maya1-secrets")],
)
@modal.asgi_app()
def maya1_tts_app():- Remove or simplify the dotenv logic:
- import os
- from dotenv import load_dotenv
-
- load_dotenv()
-
- # If not overridden, use HF repo as model path
- os.environ.setdefault("MAYA1_MODEL_PATH", "maya-research/maya1")
-
- # If the model is gated and you need auth, you can also set:
- # os.environ.setdefault("HUGGINGFACE_HUB_TOKEN", "<your_hf_token>")
+ import os
+
+ # Secrets are automatically injected as environment variables
+ # Set defaults only if not provided
+ os.environ.setdefault("MAYA1_MODEL_PATH", "maya-research/maya1")🤖 Prompt for AI Agents
In maya_modal.py around line 62, the call to load_dotenv() is inappropriate
because Modal containers won’t have a .env file; replace this with Modal
Secrets: remove or disable load_dotenv() and any reliance on dotenv, create
Modal secrets for the required variables (e.g., MAYA1_MODEL_PATH,
HUGGINGFACE_HUB_TOKEN), update the Modal function decorator to include
secrets=<your_secret_name> so the secrets are injected into the container, and
read values from os.environ at runtime; also remove the dotenv import if no
longer used.
1 participant
If you want to deploy api FastAPI-based server on modal, you can do "modal deploy maya_deploy.py", and you will get an endpoint that you can use to call your fast api server.
Summary by CodeRabbit