Add /upload_sample route to allow upload new speakers using Web API #80
Conversation
…d new speakers using API * Added python-multipart to requirements
WalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Conf as conf.sh
participant FS as Filesystem
User->>Conf: run conf.sh
Conf->>FS: check /home/dwemer/python-tts exists
Conf->>FS: list /home/dwemer/xtts-api-server/start-*.sh
Conf->>User: show numbered menu (0 = disable)
User->>Conf: select option
Conf->>FS: create/update or remove symlink /home/dwemer/xtts-api-server/start.sh
Conf->>User: confirm selection / exit on error
sequenceDiagram
participant User
participant Installer as ddistro_install.sh
participant Venv as PythonEnv
participant FS as Filesystem
participant Conf as conf.sh
participant Server as xtts_api_server
User->>Installer: run installer
Installer->>FS: cd to server dir
Installer->>Venv: create & activate venv (/home/dwemer/python-tts)
Installer->>User: prompt for GPU/CUDA choice
User->>Installer: choose GPU/CUDA option
Installer->>Venv: install tooling & requirements (select Torch wheel), apply file patch
Installer->>FS: copy assets (e.g., TheNarrator.wav)
Installer->>Conf: invoke conf.sh to set start.sh symlink
Installer->>Server: start xtts_api_server (--listen, optional --deepspeed) and tail logs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
Outside diff range and nitpick comments (2)
xtts_api_server/server.py (2)
Line range hint
195-195: Improve exception handling by using exception chaining.- raise HTTPException(status_code=400, detail=str(e)) + raise HTTPException(status_code=400, detail=str(e)) from eAlso applies to: 204-204, 224-224, 233-233, 298-298, 329-329, 353-353
Tools
Ruff
1-1:
TTS.api.TTSimported but unused (F401)Remove unused import:
TTS.api.TTS
Line range hint
150-150: Avoid redefining theget_speakersfunction.- @app.get("/speakers") + @app.get("/special_speakers") def get_speakers_special():Tools
Ruff
1-1:
TTS.api.TTSimported but unused (F401)Remove unused import:
TTS.api.TTS
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- requirements.txt (1 hunks)
- xtts_api_server/server.py (2 hunks)
Files skipped from review due to trivial changes (1)
- requirements.txt
Additional context used
Ruff
xtts_api_server/server.py
1-1:
TTS.api.TTSimported but unused (F401)Remove unused import:
TTS.api.TTS
10-10:
timeimported but unused (F401)Remove unused import:
time
12-12:
shutilimported but unused (F401)Remove unused import:
shutil
14-14:
argparse.ArgumentParserimported but unused (F401)Remove unused import:
argparse.ArgumentParser
15-15: Redefinition of unused
Pathfrom line 11 (F811)Remove definition:
Path
150-150: Redefinition of unused
get_speakersfrom line 145 (F811)
195-195: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
204-204: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
207-207: Do not perform function call
Filein argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
224-224: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
233-233: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
298-298: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
329-329: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
353-353: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
| @@ -1,5 +1,5 @@ | |||
| from TTS.api import TTS | |||
| from fastapi import BackgroundTasks, FastAPI, HTTPException, Request, Query | |||
| from fastapi import BackgroundTasks, FastAPI, HTTPException, Request, Query, File, UploadFile | |||
There was a problem hiding this comment.
Remove unused imports to clean up the code.
- from TTS.api import TTS
- import time
- import shutil
- from argparse import ArgumentParser
- from pathlib import PathCommittable suggestion was skipped due to low confidence.
| raise HTTPException(status_code=400, detail=str(e)) | ||
|
|
||
| @app.post("/upload_sample") | ||
| async def upload_sample(wavFile: UploadFile = File(...)): |
There was a problem hiding this comment.
Avoid using File(...) as a default argument.
- async def upload_sample(wavFile: UploadFile = File(...)):
+ async def upload_sample(wavFile: UploadFile = None):
+ if wavFile is None:
+ wavFile = File(...)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def upload_sample(wavFile: UploadFile = File(...)): | |
| async def upload_sample(wavFile: UploadFile = None): | |
| if wavFile is None: | |
| wavFile = File(...) |
Tools
Ruff
207-207: Do not perform function call
Filein argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
|
Great, thanks, I'll take a look tomorrow. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
requirements.txt (1)
1-169: Consider reducing dependency bloatThe requirements file includes many packages that seem unnecessary for the upload_sample route functionality:
- Multiple TTS engines (coqui-tts, pyttsx3)
- Various language processing tools (gruut, spacy, stanza)
- Multiple audio processing libraries
Consider maintaining separate requirement files:
requirements.txtfor core dependenciesrequirements-dev.txtfor development dependenciesrequirements-optional.txtfor optional featuresExample structure:
# requirements.txt (core dependencies) fastapi==0.109.0 python-multipart==0.0.20 python-dotenv==1.0.1 uvicorn==0.34.0 # requirements-optional.txt (TTS-specific dependencies) coqui-tts==0.24.1 librosa==0.10.2.post1 torch==2.5.1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
requirements.txt(1 hunks)
🔇 Additional comments (3)
requirements.txt (3)
122-122: LGTM! Required package for file upload functionalityThe addition of
python-multipartis necessary for handling file uploads in FastAPI.
38-38: Update FastAPI to the latest stable versionThe current FastAPI version (0.115.6) is outdated and may have security vulnerabilities.
Run this script to check for security advisories:
88-99: Verify CUDA compatibility with the systemThe inclusion of CUDA 12.x dependencies suggests GPU usage. Ensure these versions are compatible with:
- The installed NVIDIA drivers
- The GPU hardware
- The operating system
Run this script to check CUDA compatibility:
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
start-deepspeed.sh (1)
7-7: Optionally capture and log the server PID.
Storing the background process PID can help with service management (stopping, monitoring). For example:- python -m xtts_api_server --deepspeed --listen &> log.txt & + python -m xtts_api_server --deepspeed --listen &> log.txt & + echo $! > server.pidstart-regular.sh (1)
1-10: Consider consolidating start scripts to reduce duplication.
All three start scripts follow the same pattern differing only by the flag (--listen,--lowvram,--deepspeed). You could merge them into a singlestart.shthat accepts a mode parameter (e.g.,./start.sh lowvram), reducing maintenance overhead.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
conf.sh (2)
1-2: Enable strict error handling for robustness.
Addset -euo pipefailafter the shebang to catch failures early and avoid undefined variables.#!/bin/bash + set -euo pipefail clear
48-48: Quote variable expansions in file operations.
Unquoted paths can break if they contain spaces or special characters. Wrap expansions in quotes.- rm /home/dwemer/xtts-api-server/start.sh &>/dev/null + rm "/home/dwemer/xtts-api-server/start.sh" &>/dev/null ... - ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh + ln -sf "$selected_file" "/home/dwemer/xtts-api-server/start.sh"Also applies to: 62-62
ddistro_install.sh (1)
19-19: Remove redundant virtual environment activation.
The script sources the same venv twice (lines 4 and here). Dropping the secondsourcewill simplify the flow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
conf.sh(1 hunks)ddistro_install.sh(1 hunks)start-deepspeed.sh(1 hunks)start-lowvram.sh(1 hunks)start-regular.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
conf.sh
[error] 21-21: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
ddistro_install.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 2-2: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
start-deepspeed.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
start-lowvram.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
start-regular.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (1)
ddistro_install.sh (1)
15-15: Verify the complexsedsubstitution.
This in-place edit is brittle and may break if upstream formatting changes. Please test thissedinvocation against the target file or consider using a more robust approach (e.g., a Python script with clearer parsing).
| cd /home/dwemer/xtts-api-server/ | ||
|
|
||
| source /home/dwemer/python-tts/bin/activate |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to directory change and dynamic path resolution.
Using a hard-coded cd without checking its success can lead to silent failures. Also, absolute paths reduce portability.
Apply this diff:
- cd /home/dwemer/xtts-api-server/
+ BASE_DIR="$(dirname "$0")"
+ cd "${BASE_DIR}" || { echo "Error: failed to change directory to ${BASE_DIR}"; exit 1; }🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| cd /home/dwemer/xtts-api-server/ | ||
|
|
||
| source /home/dwemer/python-tts/bin/activate |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure cd succeeds and use script-relative paths.
The cd command should exit on failure to prevent subsequent commands from running in an unexpected directory. Also, deriving the path dynamically makes the script more portable.
- cd /home/dwemer/xtts-api-server/
+ BASE_DIR="$(dirname "$0")"
+ cd "${BASE_DIR}" || { echo "Error: could not enter ${BASE_DIR}"; exit 1; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd /home/dwemer/xtts-api-server/ | |
| source /home/dwemer/python-tts/bin/activate | |
| BASE_DIR="$(dirname "$0")" | |
| cd "${BASE_DIR}" || { echo "Error: could not enter ${BASE_DIR}"; exit 1; } | |
| source /home/dwemer/python-tts/bin/activate |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| cd /home/dwemer/xtts-api-server/ | ||
|
|
||
| source /home/dwemer/python-tts/bin/activate |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error checking on directory change and derive path from script location.
This prevents silent failures if the directory is missing and makes the script agnostic of a fixed install path.
- cd /home/dwemer/xtts-api-server/
+ BASE_DIR="$(dirname "$0")"
+ cd "${BASE_DIR}" || { echo "Error: failed to change directory to ${BASE_DIR}"; exit 1; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd /home/dwemer/xtts-api-server/ | |
| source /home/dwemer/python-tts/bin/activate | |
| BASE_DIR="$(dirname "$0")" | |
| cd "${BASE_DIR}" || { echo "Error: failed to change directory to ${BASE_DIR}"; exit 1; } | |
| source /home/dwemer/python-tts/bin/activate |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| if [ ! -d /home/dwemer/python-tts ]; then | ||
| exit "XTTSv2 not installed" | ||
| fi |
There was a problem hiding this comment.
Fix invalid use of exit with a string.
exit only accepts numeric status codes. Use echo to print the message and then exit 1.
- if [ ! -d /home/dwemer/python-tts ]; then
- exit "XTTSv2 not installed"
+ if [ ! -d /home/dwemer/python-tts ]; then
+ echo "Error: XTTSv2 not installed" >&2
+ exit 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ ! -d /home/dwemer/python-tts ]; then | |
| exit "XTTSv2 not installed" | |
| fi | |
| if [ ! -d /home/dwemer/python-tts ]; then | |
| echo "Error: XTTSv2 not installed" >&2 | |
| exit 1 | |
| fi |
🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 21-21: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
| @@ -0,0 +1,34 @@ | |||
|
|
|||
| cd /home/dwemer/xtts-api-server | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure cd command is checked.
If the directory change fails, the rest of the script will run in the wrong context. Wrap it with an exit on failure:
- cd /home/dwemer/xtts-api-server
+ cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to xtts-api-server"; exit 1; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd /home/dwemer/xtts-api-server | |
| cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to xtts-api-server"; exit 1; } |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 2-2: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
|
|
||
| cd /home/dwemer/xtts-api-server |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add shebang and strict error handling.
There is no shebang at the top, and failures won't be caught. Prepend these lines:
+ #!/bin/bash
+ set -euo pipefail🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 2-2: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
ddistro_install.sh (2)
1-1: Add shebang and strict mode at the top of the script.Without a shebang and strict error settings, the script’s behavior is undefined and errors may not be caught early. Add at the top:
#!/bin/bash set -euo pipefail🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
2-2: Guard directory change with error handling.If the
cdfails, subsequent commands will execute in the wrong context. Update to:- cd /home/dwemer/xtts-api-server + cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to xtts-api-server"; exit 1; }🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 2-2: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🧹 Nitpick comments (2)
ddistro_install.sh (2)
8-10: Combine pip installation steps for clarity and performance.Instead of multiple invocations, you can group upgrades and requirements:
- python3 -m pip install --upgrade pip wheel ninja virtualenv - pip install setuptools==68.1.2 - pip install -r requirements.txt + pip install --upgrade pip wheel ninja virtualenv setuptools==68.1.2 + pip install -r requirements.txtThis reduces overhead and improves readability.
19-19: Remove redundant virtual environment activation.The venv is already activated at line 4. Activating it again is unnecessary and could mask activation failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
ddistro_install.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 2-2: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
|
|
||
| sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py | ||
|
|
||
| cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav |
There was a problem hiding this comment.
Check file existence before copying speaker sample.
Copying /home/dwemer/TheNarrator.wav without verifying it exists will fail silently or with an unclear error. Add a guard:
if [[ ! -f "$SAMPLE_PATH" ]]; then
echo "Error: sample file not found at $SAMPLE_PATH"
exit 1
fi
cp "$SAMPLE_PATH" "${BASE_DIR}/speakers/"| cd /home/dwemer/xtts-api-server | ||
| python3 -m venv /home/dwemer/python-tts | ||
| source /home/dwemer/python-tts/bin/activate |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Parameterize paths to improve reusability.
Hard-coding /home/dwemer and the venv path makes the script user- and environment-specific. Consider:
BASE_DIR="${BASE_DIR:-$HOME/xtts-api-server}"
VENV_DIR="${VENV_DIR:-$HOME/python-tts}"
cd "$BASE_DIR" || exit 1
python3 -m venv "$VENV_DIR"
source "$VENV_DIR/bin/activate"This allows users to override paths without editing the script.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 2-2: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
conf.sh (1)
21-23: Fix invalid use of exit with a string.
exitonly accepts numeric status codes. Useechoto print the error message and thenexit 1. This matches the ShellCheck hint (SC2242) and the previous review feedback.if [ ! -d /home/dwemer/python-tts ]; then - exit "XTTSv2 not installed" + echo "Error: XTTSv2 not installed" >&2 + exit 1 fi🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 22-22: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
🧹 Nitpick comments (4)
conf.sh (4)
34-34: Correct the menu prompt typo.There's a duplicated article in the prompt. Update
"Select a an option"to"Select an option".-echo -e "Select a an option from the list:\n\n" +echo -e "Select an option from the list:\n\n"
47-51: Avoid numeric comparison on unvalidated input.Using
-eqon a non-numericselectioncan cause an error if the user inputs a non-integer. Either validate thatselectionis numeric before this check, or switch to string comparison (== "0").-if [ "$selection" -eq "0" ]; then +if [[ "$selection" == "0" ]]; thenOptionally, you can move this block after the numeric-validation block at lines 53–56.
63-63: Quote variables inlninvocation.Unquoted paths can break if
$selected_filecontains spaces or special characters. Wrap both source and target in quotes.-ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh +ln -sf "$selected_file" "/home/dwemer/xtts-api-server/start.sh"
21-26: Use variables for installation paths.Hard-coded
/home/dwemer/...paths reduce portability. Consider defining variables at the top (e.g.,BASE_DIRandTTS_DIR) or using$HOME:+BASE_DIR="${HOME}/xtts-api-server" +TTS_DIR="${HOME}/python-tts" if [ ! -d /home/dwemer/python-tts ]; then - echo "Error: XTTSv2 not installed" >&2 - exit 1 + echo "Error: XTTSv2 not installed" >&2 + exit 1 fi -mapfile -t files < <(find /home/dwemer/xtts-api-server/ -name "start-*.sh") +mapfile -t files < <(find "$BASE_DIR" -name "start-*.sh")🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 22-22: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
conf.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
conf.sh
[error] 22-22: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
conf.sh (1)
19-21:⚠️ Potential issueFix invalid use of
exitwith a string
Theexitbuilt-in only accepts integer status codes. Writing an error message to stderr and then exiting with a non-zero code will correctly signal failure.if [ ! -d /home/dwemer/python-tts ]; then - exit "XTTSv2 not installed" + echo "Error: XTTSv2 not installed" >&2 + exit 1 fi🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 20-20: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
🧹 Nitpick comments (1)
conf.sh (1)
61-61: Quote the variable in thelncommand
Unquoted variables can break if the path contains whitespace or glob characters.-ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh +ln -sf "$selected_file" /home/dwemer/xtts-api-server/start.sh
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
conf.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
conf.sh
[error] 20-20: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
conf.sh (1)
19-21: Fix invalid use ofexitwith a string
exitonly accepts numeric status codes. Errors should be written to stderr and then exit with a non-zero status code.- if [ ! -d /home/dwemer/python-tts ]; then - exit "XTTSv2 not installed" - fi + if [ ! -d /home/dwemer/python-tts ]; then + echo "Error: XTTSv2 not installed" >&2 + exit 1 + fi🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 20-20: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
🧹 Nitpick comments (2)
conf.sh (2)
32-32: Typo in the selection prompt
There’s a duplicate article in the prompt"Select a an option...". It should read"Select an option...".- echo -e "Select a an option from the list:\n\n" + echo -e "Select an option from the list:\n\n"
23-23: Avoid hard-coded absolute paths
Consider defining a base directory variable (e.g.,XTTS_HOME="${XTTS_HOME:-$HOME/dwemer}") at the top and referencing it throughout. This improves portability and makes future relocations/configurations easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
conf.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
conf.sh
[error] 20-20: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
|
|
||
| echo "You selected: $selected_file" | ||
|
|
||
| ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Quote path variables in ln -sf
Always quote expansions to handle paths containing spaces or special characters:
- ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh
+ ln -sf "$selected_file" "/home/dwemer/xtts-api-server/start.sh"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh | |
| ln -sf "$selected_file" "/home/dwemer/xtts-api-server/start.sh" |
| if [ "$selection" -eq "0" ]; then | ||
| echo "Disabling service. Run this again to enable" | ||
| rm /home/dwemer/xtts-api-server/start.sh &>/dev/null | ||
| exit 0 | ||
| fi | ||
|
|
||
| if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then | ||
| echo "Invalid selection." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reorder input validation to prevent comparison errors
Using -eq on a non-numeric string will cause a runtime error. Check for numeric input first or use string comparison for the zero case. For example:
- if [ "$selection" -eq "0" ]; then
+ # Check for zero before numeric comparison
+ if [ "$selection" = "0" ]; then
echo "Disabling service. Run this again to enable"
rm /home/dwemer/xtts-api-server/start.sh &>/dev/null
exit 0
fi
- if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then
+ if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then
echo "Invalid selection."
exit 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ "$selection" -eq "0" ]; then | |
| echo "Disabling service. Run this again to enable" | |
| rm /home/dwemer/xtts-api-server/start.sh &>/dev/null | |
| exit 0 | |
| fi | |
| if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then | |
| echo "Invalid selection." | |
| exit 1 | |
| fi | |
| # Check for zero before numeric comparison | |
| if [ "$selection" = "0" ]; then | |
| echo "Disabling service. Run this again to enable" | |
| rm /home/dwemer/xtts-api-server/start.sh &>/dev/null | |
| exit 0 | |
| fi | |
| if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then | |
| echo "Invalid selection." | |
| exit 1 | |
| fi |
* Added branch for Nvidia GT 10xx users (will use cuda 118)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
ddistro_install.sh (5)
1-2: Add strict error handling flags
To catch errors and undefined variables early, addset -euo pipefailright after the shebang.Suggested diff:
#!/bin/bash + set -euo pipefail
3-3: Validatecdsuccess
Ifcdfails, the script will continue in the wrong directory. Append an exit handler to guard against that.Suggested diff:
- cd /home/dwemer/xtts-api-server + cd /home/dwemer/xtts-api-server || { echo "Error: failed to cd to xtts-api-server"; exit 1; }
4-5: Parameterize hard-coded paths
Using fixed paths reduces portability. IntroduceBASE_DIRandVENV_DIRvariables so users can override them.Example:
- python3 -m venv /home/dwemer/python-tts - source /home/dwemer/python-tts/bin/activate + BASE_DIR="${BASE_DIR:-$HOME/xtts-api-server}" + VENV_DIR="${VENV_DIR:-$HOME/python-tts}" + + python3 -m venv "$VENV_DIR" + source "$VENV_DIR/bin/activate" + cd "$BASE_DIR" || exit 1
24-24: Avoid brittle in-placesededits
Patching installed packages withsedcan break on updates. Prefer delivering a proper patch or at least guard the file’s existence before editing.Example check:
FILE="/home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py" if [[ -f "$FILE" ]]; then sed -i 's/.../.../' "$FILE" else echo "Error: $FILE not found"; exit 1 fi
26-26: Ensure speaker sample exists before copying
Copying without checking the source file can fail with an unclear error. Add a guard to verify the path.Suggested diff:
- cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav + SAMPLE_PATH="${SAMPLE_PATH:-/home/dwemer/TheNarrator.wav}" + if [[ ! -f "$SAMPLE_PATH" ]]; then + echo "Error: speaker sample not found at $SAMPLE_PATH"; exit 1 + fi + cp "$SAMPLE_PATH" "speakers/TheNarrator.wav"
🧹 Nitpick comments (4)
ddistro_install.sh (4)
10-15: Handle invalid input in GPU prompt
Currently any response other than “yes” falls back silently to the non-GPU path. Use a loop orcaseto re-prompt until valid.Example:
while true; do read -p "Do you own a GT10XX series GPU? (yes/no): " ans case "$ans" in [Yy]|[Yy][Ee][Ss]) torch_url="https://download.pytorch.org/whl/nightly/cu118"; break ;; [Nn]|[Nn][Oo]) torch_url="https://download.pytorch.org/whl/nightly/cu128"; break ;; *) echo "Please answer yes or no." ;; esac done
17-21: Consistent invocation of pip
Mixingpython3 -m pipand barepipcan install into different environments. Stick to one form—preferpython3 -m pip.Suggested diff:
- pip install setuptools==68.1.2 - pip install -r requirements.txt - pip uninstall -y torch torchaudio - pip install --pre -U torch torchaudio --index-url "$torch_url" + python3 -m pip install setuptools==68.1.2 + python3 -m pip install -r requirements.txt + python3 -m pip uninstall -y torch torchaudio + python3 -m pip install --pre -U torch torchaudio --index-url "$torch_url"
30-30: Verifyconf.shexecution
Ifconf.shis missing or not executable, the script will error out later. Add a pre-check.Example:
if [[ ! -x "./conf.sh" ]]; then echo "Error: conf.sh not found or not executable"; exit 1 fi ./conf.sh
40-40: Use consistent Python interpreter for server launch
To ensure the correct venv is used, explicitly invoke the venv’s Python.Suggested diff:
- python -m xtts_api_server --listen + "$VENV_DIR/bin/python" -m xtts_api_server --listen
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
ddistro_install.sh
[error] 42-42: Couldn't parse this double quoted string. Fix to allow more checks.
(SC1073)
* Fixed unclosed "
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
ddistro_install.sh (5)
1-2: Enable strict error handling for robustness.The script currently has a shebang but lacks strict mode, which can lead to silent failures or use of unset variables. Add
set -euo pipefailafter the shebang to ensure it exits on errors and undefined variables.
3-3: Parameterize and validate the working directory change.Avoid hard-coding
/home/dwemer/xtts-api-server. Introduce aBASE_DIRvariable (with a sensible default) and guard thecd:- cd /home/dwemer/xtts-api-server + BASE_DIR="${BASE_DIR:-/home/dwemer/xtts-api-server}" + cd "$BASE_DIR" || { echo "Error: cannot cd to $BASE_DIR"; exit 1; }🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
4-5: Refactor virtual environment setup to use configurable paths.Hard-coded VENV location reduces reusability. Use a
VENV_DIRvariable and reference it consistently:- python3 -m venv /home/dwemer/python-tts - source /home/dwemer/python-tts/bin/activate + VENV_DIR="${VENV_DIR:-$HOME/python-tts}" + python3 -m venv "$VENV_DIR" + source "$VENV_DIR/bin/activate"
24-24: Guard against missingxtts.pybefore applying in-place patch.Editing site-packages in place is brittle. At minimum, verify the file exists and consider using a patch file or upstream PR:
+ TTS_MODEL_FILE="$VENV_DIR/lib/python$(python3 -c 'import sys; print(f"{sys.version_info.major}.{sys.version_info.minor}")')/site-packages/TTS/tts/models/xtts.py" + if [[ -f "$TTS_MODEL_FILE" ]]; then + sed -i 's/checkpoint = load_fsspec(...)/checkpoint = load_fsspec(..., weights_only=False)/' "$TTS_MODEL_FILE" + else + echo "Error: $TTS_MODEL_FILE not found" >&2 + exit 1 + fi
26-26: Verify speaker sample file existence before copying.Directly copying
/home/dwemer/TheNarrator.wavcan fail without a clear message. ParameterizeSAMPLE_PATH, ensure the directory exists, and guard the copy:+ SAMPLE_PATH="${SAMPLE_PATH:-/home/dwemer/TheNarrator.wav}" + DEST_DIR="speakers" + if [[ ! -f "$SAMPLE_PATH" ]]; then + echo "Error: sample file '$SAMPLE_PATH' not found" >&2 + exit 1 + fi + mkdir -p "$DEST_DIR" + cp "$SAMPLE_PATH" "$DEST_DIR/"
🧹 Nitpick comments (3)
ddistro_install.sh (3)
10-15: Enhance GPU selection prompt validation (optional).The regex handles basic yes/no but doesn’t catch invalid inputs. Consider normalizing to lowercase and using a
casestatement with a default branch:read -p "Do you own a GT10XX series GPU? (yes/no): " gpu_answer gpu_answer="${gpu_answer,,}" case "$gpu_answer" in y|yes) torch_url="https://download.pytorch.org/whl/nightly/cu118" ;; n|no) torch_url="https://download.pytorch.org/whl/nightly/cu128" ;; *) echo "Invalid response: $gpu_answer"; exit 1 ;; esac
22-22: Remove or fix the commented-out command.The
#pip install xtts-api-serverline either needs context (why it fails) or should be removed to keep the script clean.
28-31: Ensureconf.shexecution is validated.If
conf.shfails or is missing, the script should stop and report an error:+ if [[ ! -f "./conf.sh" ]]; then + echo "Error: conf.sh not found"; exit 1 + fi + ./conf.sh || { echo "Error: conf.sh failed"; exit 1; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
ddistro_install.sh
Outdated
| python3 -m pip install --upgrade pip wheel ninja virtualenv | ||
| pip install setuptools==68.1.2 | ||
| pip install -r requirements.txt | ||
| pip uninstall -y torch torchaudio | ||
| pip install --pre -U torch torchaudio --index-url "$torch_url" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consistently invoke pip via python3 -m pip within the venv.
Mixing bare pip and python3 -m pip can lead to ambiguity about which interpreter is used. Update all install/uninstall commands:
- pip install setuptools==68.1.2
- pip install -r requirements.txt
- pip uninstall -y torch torchaudio
- pip install --pre -U torch torchaudio --index-url "$torch_url"
+ python3 -m pip install setuptools==68.1.2
+ python3 -m pip install -r requirements.txt
+ python3 -m pip uninstall -y torch torchaudio
+ python3 -m pip install --pre -U torch torchaudio --index-url "$torch_url"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| python3 -m pip install --upgrade pip wheel ninja virtualenv | |
| pip install setuptools==68.1.2 | |
| pip install -r requirements.txt | |
| pip uninstall -y torch torchaudio | |
| pip install --pre -U torch torchaudio --index-url "$torch_url" | |
| python3 -m pip install --upgrade pip wheel ninja virtualenv | |
| python3 -m pip install setuptools==68.1.2 | |
| python3 -m pip install -r requirements.txt | |
| python3 -m pip uninstall -y torch torchaudio | |
| python3 -m pip install --pre -U torch torchaudio --index-url "$torch_url" |
ddistro_install.sh
Outdated
| python -m xtts_api_server --listen | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use the venv’s Python interpreter to launch the server.
Relying on system python may bypass the virtual environment. Switch to python3 (or $VENV_DIR/bin/python):
- python -m xtts_api_server --listen
+ python3 -m xtts_api_server --listen📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| python -m xtts_api_server --listen | |
| -python -m xtts_api_server --listen | |
| +python3 -m xtts_api_server --listen |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
start-deepspeed.sh (1)
7-7: Ensurecdsucceeds and use script-relative paths.As flagged by ShellCheck (SC2164) and previously noted in past reviews, use the script's directory dynamically and exit if
cdfails.-cd /home/dwemer/xtts-api-server/ +BASE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/.." +cd "${BASE_DIR}" || { echo "Error: could not change directory to ${BASE_DIR}" >&2; exit 1; }🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🧹 Nitpick comments (2)
start-deepspeed.sh (2)
1-1: Enable strict mode for safer error handling.Add
set -euo pipefailafter the shebang to ensure the script exits on any error, catches unset variables, and fails on pipeline errors.#!/bin/bash + set -euo pipefail
9-9: Avoid hardcoding virtual environment paths.Make the virtual environment activation path configurable or relative to the script location to improve portability. For example:
-source /home/dwemer/python-tts/bin/activate +source "${BASE_DIR}/python-tts/bin/activate"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
start-deepspeed.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
start-deepspeed.sh
[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
ddistro_install.sh (6)
1-2: Enable strict mode for safer scripting.Add fail-fast and safer IFS. This prevents subtle errors from continuing unnoticed.
#!/bin/bash + +set -euo pipefail +# Safer word splitting for arrays/paths +IFS=$'\n\t'
3-5: Parameterize paths and guard cd; improve portability and reliability.Avoid hardcoded user paths and ensure directory change succeeds. This also addresses SC2164 from shellcheck.
-cd /home/dwemer/xtts-api-server -python3 -m venv /home/dwemer/python-tts -source /home/dwemer/python-tts/bin/activate +BASE_DIR="${BASE_DIR:-$HOME/xtts-api-server}" +VENV_DIR="${VENV_DIR:-$HOME/python-tts}" +cd "$BASE_DIR" || { echo "Error: cannot cd to $BASE_DIR"; exit 1; } +python3 -m venv "$VENV_DIR" +# shellcheck disable=SC1091 +source "$VENV_DIR/bin/activate"
17-23: Use the venv interpreter for all pip ops; avoid interpreter ambiguity.Standardize on python -m pip and keep the current ordering if you intend the nightly torch stack to override any torch specified in requirements.
python3 -m pip install --upgrade pip wheel ninja virtualenv -pip install setuptools==68.1.2 -pip install -r requirements.txt +python3 -m pip install setuptools==68.1.2 +python3 -m pip install -r requirements.txt # Ensure fresh installs of PyTorch stack including TorchCodec required by recent torchaudio -pip uninstall -y torch torchaudio torchcodec -pip install --pre -U torch torchaudio torchcodec --index-url "$torch_url" +python3 -m pip uninstall -y torch torchaudio torchcodec || true +python3 -m pip install --pre -U torch torchaudio torchcodec --index-url "$torch_url"Optional: If requirements.txt also pins torch/torchaudio, consider removing them there to avoid churn, or install requirements with
--no-depsafter the torch stack. Please confirm the desired precedence.
25-25: Avoid brittle sed patching of site-packages; compute path dynamically and back up.Hardcoding Python 3.11 and direct in-place edits are fragile and will break on upgrades or different environments. Prefer a targeted patch with backup based on the actual installed path.
-sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py +# Locate installed xtts.py and patch safely +XTTS_FILE="$(python3 - <<'PY' +import inspect, TTS.tts.models.xtts as m +print(inspect.getfile(m)) +PY +)" +if [[ -f "$XTTS_FILE" ]]; then + cp "$XTTS_FILE" "${XTTS_FILE}.bak" + python3 - "$XTTS_FILE" <<'PY' +import sys, pathlib +p = pathlib.Path(sys.argv[1]) +s = p.read_text() +old = 'load_fsspec(model_path, map_location=torch.device("cpu"))["model"]' +new = 'load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]' +if old in s and new not in s: + p.write_text(s.replace(old, new)) +else: + print("Note: expected pattern not found or already patched", file=sys.stderr) +PY +else + echo "Error: xtts.py not found; cannot patch." >&2 + exit 1 +fiLong-term, prefer upstreaming this change or applying a proper patch file rather than mutating installed packages.
27-27: Validate sample file and ensure speakers dir exists before copying.Prevents confusing failures on missing source file or directory.
-cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav +SAMPLE_PATH="${SAMPLE_PATH:-$HOME/TheNarrator.wav}" +mkdir -p speakers +if [[ ! -f "$SAMPLE_PATH" ]]; then + echo "Error: sample file not found at $SAMPLE_PATH" >&2 + exit 1 +fi +cp "$SAMPLE_PATH" "speakers/TheNarrator.wav"
41-41: Use the venv’s Python interpreter to run the server.Guarantees the correct environment and dependencies.
-python -m xtts_api_server --listen +"${VENV_DIR:-$HOME/python-tts}"/bin/python -m xtts_api_server --listen
🧹 Nitpick comments (3)
ddistro_install.sh (3)
10-15: Make CUDA index selection robust; optionally auto-detect GPU capability.The yes/no prompt works but is brittle. Consider auto-detecting compute capability via nvidia-smi and mapping to the correct CUDA wheel, with a manual fallback.
-read -p "Do you own a GT10XX series GPU? (yes/no): " gpu_answer -if [[ "$gpu_answer" =~ ^[Yy][Ee][Ss]$ || "$gpu_answer" =~ ^[Yy]$ ]]; then - torch_url="https://download.pytorch.org/whl/nightly/cu118" -else - torch_url="https://download.pytorch.org/whl/nightly/cu128" -fi +# Try to detect CUDA capability; fall back to prompt +torch_url="" +if command -v nvidia-smi >/dev/null 2>&1; then + cc="$(nvidia-smi --query-gpu=compute_cap --format=csv,noheader 2>/dev/null | head -n1 || true)" + case "$cc" in + 6.*) torch_url="https://download.pytorch.org/whl/nightly/cu118" ;; # Pascal + 7.*|8.*|9.*) torch_url="https://download.pytorch.org/whl/nightly/cu128" ;; # Turing/Ampere+/Blackwell + esac +fi +if [[ -z "${torch_url}" ]]; then + read -r -p "Use CUDA 11.8 wheels? [y/N]: " ans + if [[ "$ans" =~ ^[Yy]$ ]]; then torch_url="https://download.pytorch.org/whl/nightly/cu118"; else torch_url="https://download.pytorch.org/whl/nightly/cu128"; fi +fi +echo "Using PyTorch nightly index: $torch_url"Note: Please verify the mapping for your supported GPUs and desired CUDA runtime.
31-31: Ensure conf.sh exists and is executable; fail fast if not.Avoids silent no-ops or confusing errors later.
-./conf.sh +if [[ ! -x ./conf.sh ]]; then + echo "Error: ./conf.sh missing or not executable" >&2 + exit 1 +fi +./conf.sh
43-44: Either remove the trailing prompt or read input.Right now it only prints “Press Enter” without waiting. If you intend to pause, add a read.
-echo "Press Enter" +read -r -p "Press ENTER to exit... " _
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (1)
ddistro_install.sh (1)
1-44: Confirm PR scope alignment.PR objectives mention adding a new /upload_sample API route and python-multipart dependency. This script is an installation/startup helper and does not relate to that route. Verify whether this file belongs in this PR or should be split into a separate infra/setup PR to keep changes focused.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
ddistro_install.sh (8)
1-1: Add strict/defensive shell options right after the shebang.Enable fail-fast behavior and safer word-splitting to avoid subtle breakages during long installs.
#!/bin/bash +set -euo pipefail +IFS=$'\n\t' +# Optional: surface a helpful message on unexpected errors +trap 'echo "Error on line ${LINENO}. Exiting." >&2' ERR
3-5: Parameterize hard-coded paths for portability.Make the script reusable across users/machines and easier to override in CI.
- cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to /home/dwemer/xtts-api-server"; exit 1; } - python3 -m venv /home/dwemer/python-tts - source /home/dwemer/python-tts/bin/activate + BASE_DIR="${BASE_DIR:-$HOME/xtts-api-server}" + VENV_DIR="${VENV_DIR:-$HOME/python-tts}" + cd "${BASE_DIR}" || { echo "Error: cannot cd to ${BASE_DIR}"; exit 1; } + python3 -m venv "${VENV_DIR}" + # shellcheck source=/dev/null + source "${VENV_DIR}/bin/activate" @@ - source /home/dwemer/python-tts/bin/activate +# shellcheck source=/dev/null + source "${VENV_DIR}/bin/activate"Also applies to: 40-40
36-36: Avoid brittle in-place sed patching of site-packages; compute site-packages dynamically.The hard-coded Python 3.11 path will break on other Python versions or layouts. Also consider carrying a patch file or upstreaming the change.
-sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py +# Derive site-packages path from the active venv +SITE_PKGS="$(python3 -c 'import site,sys; print((site.getsitepackages() or [site.getusersitepackages()])[0])')" +TARGET_FILE="${SITE_PKGS}/TTS/tts/models/xtts.py" +if [[ -f "${TARGET_FILE}" ]]; then + sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' "${TARGET_FILE}" +else + echo "Warning: ${TARGET_FILE} not found; skip patch." >&2 +fiIf you’d like, I can prepare a minimal patch file and apply it via patch -p0 for better traceability.
38-38: Guard the sample copy with an existence check (and use parameterized paths).Prevents confusing failures if the file is missing.
-cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav +SAMPLE_PATH="${SAMPLE_PATH:-$HOME/TheNarrator.wav}" +if [[ ! -f "${SAMPLE_PATH}" ]]; then + echo "Error: sample file not found at ${SAMPLE_PATH}" >&2 + exit 1 +fi +mkdir -p "${BASE_DIR:-$PWD}/speakers" +cp "${SAMPLE_PATH}" "${BASE_DIR:-$PWD}/speakers/TheNarrator.wav"
3-3: Check the result of cd to prevent running in the wrong directory.If cd fails, later operations will patch/modify the wrong environment.
-cd /home/dwemer/xtts-api-server +cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to /home/dwemer/xtts-api-server"; exit 1; }
23-33: Use the venv’s interpreter for all pip actions; avoid mixing with bare pip.Prevents accidental use of system pip/interpreter and ambiguity.
-python3 -m pip install --upgrade pip wheel ninja virtualenv -pip install setuptools==68.1.2 -# Install app requirements without auto-pulling torch/torchaudio from deps -pip install --no-deps -r requirements.txt -# Pin to stable, CUDA-tagged PyTorch/Torchaudio that do not require TorchCodec -pip cache purge || true -pip uninstall -y torch torchaudio torchcodec torchvision || true -pip install --no-deps --no-cache-dir --index-url "$torch_url" "torch==${torch_ver}+${cu_tag}" "torchaudio==${torchaudio_ver}+${cu_tag}" -pip check || true -# Ensure fallback audio loader is available -pip install --no-cache-dir soundfile +python3 -m pip install --upgrade pip wheel ninja virtualenv +python3 -m pip install setuptools==68.1.2 +# Install app requirements without auto-pulling torch/torchaudio from deps +python3 -m pip install --no-deps -r requirements.txt +# Pin to stable, CUDA-tagged PyTorch/Torchaudio that do not require TorchCodec +python3 -m pip cache purge || true +python3 -m pip uninstall -y torch torchaudio torchcodec torchvision || true +python3 -m pip install --no-deps --no-cache-dir --index-url "$torch_url" \ + "torch==${torch_ver}+${cu_tag}" "torchaudio==${torchaudio_ver}+${cu_tag}" +# Fail if dependency conflicts are detected +python3 -m pip check +# Ensure fallback audio loader is available +python3 -m pip install --no-cache-dir soundfile
44-52: Fix contradictory prompts and start the server with the venv’s Python.Current messaging asks users to wait for Uvicorn output before you actually start the server and tells them to close the window (which kills the server). Start immediately after the confirmation and remove the “close this window” instruction.
-echo -echo "This will start CHIM XTTS to download the selected model" -echo "Wait for the message 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'" -echo "Then close this window. Press ENTER to continue" -read - -echo "please wait...." - -python -m xtts_api_server --listen +echo +echo "Press ENTER to start CHIM XTTS and download the selected model." +echo "After startup, look for: 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'." +echo "Keep this terminal open to keep the server running." +read -r +echo "Starting server, please wait..." +"${VENV_DIR:-$HOME/python-tts}"/bin/python -m xtts_api_server --listenIf you need the server to persist after logout, consider adding a systemd unit, or use tmux/screen.
52-52: Launch with the venv’s interpreter to avoid escaping the environment.Using system python may miss installed deps in the venv.
-python -m xtts_api_server --listen +"${VENV_DIR:-$HOME/python-tts}"/bin/python -m xtts_api_server --listen
🧹 Nitpick comments (4)
ddistro_install.sh (4)
10-21: Harden yes/no prompt parsing and avoid backslash escapes eating with read.Use read -r and a case statement; improves UX and robustness.
-read -p "Are you using a GT10XX series GPU? (yes/no): " gpu_answer -if [[ "$gpu_answer" =~ ^[Yy][Ee][Ss]$ || "$gpu_answer" =~ ^[Yy]$ ]]; then +read -r -p "Are you using a GT10XX series GPU? (yes/no): " gpu_answer +case "${gpu_answer}" in + [Yy]|[Yy][Ee][Ss]) cu_tag="cu118" torch_url="https://download.pytorch.org/whl/${cu_tag}" torch_ver="2.2.2" torchaudio_ver="2.2.2" -else + ;; + *) cu_tag="cu124" torch_url="https://download.pytorch.org/whl/${cu_tag}" torch_ver="2.4.0" torchaudio_ver="2.4.0" -fi + ;; +esac
31-31: Don’t mask dependency conflicts with|| true.If pip check fails, the environment is inconsistent and later steps may break at runtime.
-pip check || true +python3 -m pip check
40-41: Second venv activation is redundant.You already activated earlier; re-sourcing is unnecessary unless you intentionally deactivated.
-source /home/dwemer/python-tts/bin/activate +# (Redundant) already active; remove unless deliberately re-activating after a subshell.
54-55: Align the final prompt with intended behavior.If you expect a pause, add a read; otherwise, remove the dangling prompt.
-echo "Press Enter" +echo "Setup complete." +# If you want a pause here: +# read -r -p "Press ENTER to exit..."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
ddistro_install.sh
Outdated
| #pip install xtts-api-server #Fails | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify xtts_api_server is importable before trying to run it.
requirements.txt install doesn’t show how xtts_api_server is provided (editable install, local package, etc.). Ensure it exists to avoid runtime failure.
Run this script to check for the module/package presence in the repo and potential entry-points:
Also applies to: 52-52
🏁 Script executed:
#!/bin/bash
# Search for a Python package/module named xtts_api_server
rg -nP -C2 '\bxtts_api_server\b' --type py
# Look for a setup.cfg/pyproject with "xtts_api_server" or console_scripts entrypoints
fd -H 'pyproject.toml' -x rg -nP -C2 'xtts_api_server|console_scripts'
fd -H 'setup.cfg' -x rg -nP -C2 'xtts_api_server|console_scripts'Length of output: 1773
Ensure the xtts_api_server package is installed and importable before invoking it
It looks like you have a local xtts_api_server/ module (so xtts_api_server is importable in the repo), but there’s no editable or regular install step in ddistro_install.sh, nor is there a console‐script entrypoint defined in your packaging. Without installing the package (or defining a CLI entrypoint), attempts to run xtts-api-server will fail.
• ddistro_install.sh (lines 34–35): replace the commented-out pip install line with a local install, for example:
pip install -e . # installs the local xtts_api_server package(or pip install . if you prefer a non-editable install) before you try to invoke the server.
• Alternatively, add a CLI entrypoint in your pyproject.toml so that pip install . also installs an executable:
[project.scripts]
xtts-api-server = "xtts_api_server.__main__:main"Then pip install . (or pip install -e .) will provide a xtts-api-server command.
🤖 Prompt for AI Agents
In ddistro_install.sh around lines 34-35, the script currently comments out
installing the local xtts_api_server package which means the xtts-api-server
command won’t be available; replace that commented line with a local install
(e.g., run pip install -e . or pip install .) before invoking the server, or
alternatively add a console-script entrypoint in pyproject.toml (project.scripts
mapping to xtts_api_server.__main__:main) so pip install . will create the
xtts-api-server executable.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
start-regular.sh (1)
8-8: Add error checking on cd and derive base path from script location.Prevents silent failures and avoids hardcoded user-specific paths.
- cd /home/dwemer/xtts-api-server/ + BASE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)" + cd "${BASE_DIR}" || { echo "Error: failed to change directory to ${BASE_DIR}" >&2; exit 1; }start-lowvram.sh (1)
8-8: Add error handling on cd and avoid hardcoded absolute path.Use script location for portability; exit on failure.
- cd /home/dwemer/xtts-api-server/ + BASE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)" + cd "${BASE_DIR}" || { echo "Error: failed to change directory to ${BASE_DIR}" >&2; exit 1; }conf.sh (3)
19-21: Fix invalid use of exit with a string.exit only accepts numeric codes; print message then exit with non-zero.
-if [ ! -d /home/dwemer/python-tts ]; then - exit "XTTSv2 not installed" +if [ ! -d /home/dwemer/python-tts ]; then + echo "Error: XTTSv2 not installed" >&2 + exit 1 fi
45-54: Reorder and harden input validation to avoid integer comparison on non-numeric.-if [ "$selection" -eq "0" ]; then +if [ "$selection" = "0" ]; then echo "Disabling service. Run this again to enable" rm /home/dwemer/xtts-api-server/start.sh &>/dev/null exit 0 fi -if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then +if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then echo "Invalid selection." exit 1 fi
61-61: Quote variables in ln to handle spaces and special characters.-ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh +ln -sf "$selected_file" "/home/dwemer/xtts-api-server/start.sh"
🧹 Nitpick comments (12)
start-regular.sh (4)
10-10: Guard virtualenv activation and fail fast if missing.If sourcing fails, the script continues and may run system Python with wrong deps.
-source /home/dwemer/python-tts/bin/activate +VENV_ACTIVATE="/home/dwemer/python-tts/bin/activate" +if [ -f "$VENV_ACTIVATE" ]; then + # shellcheck disable=SC1090 + source "$VENV_ACTIVATE" +else + echo "Error: venv not found at $VENV_ACTIVATE" >&2 + exit 1 +fi
1-1: Enable strict mode for safer scripting.Catch unset vars and early-fail on errors.
#!/bin/bash +set -Eeuo pipefail
4-6: Generalize CUDA PATH detection.Fallback to the default /usr/local/cuda/bin if versioned dir isn’t present.
-if [ -d "/usr/local/cuda-12.8/bin" ]; then - export PATH="/usr/local/cuda-12.8/bin:$PATH" +if [ -d "/usr/local/cuda-12.8/bin" ]; then + export PATH="/usr/local/cuda-12.8/bin:$PATH" +elif [ -d "/usr/local/cuda/bin" ]; then + export PATH="/usr/local/cuda/bin:$PATH" fi
12-15: Capture PID and optionally harden backgrounding.Persist the background PID for later management and make logging intent explicit.
-date > log.txt +date > log.txt -python -m xtts_api_server --listen &>> log.txt & +python -m xtts_api_server --listen >> log.txt 2>&1 & +echo $! > xtts.pidstart-lowvram.sh (4)
1-1: Enable strict mode for resilience.#!/bin/bash +set -Eeuo pipefail
10-10: Validate virtualenv before sourcing.-source /home/dwemer/python-tts/bin/activate +VENV_ACTIVATE="/home/dwemer/python-tts/bin/activate" +if [ -f "$VENV_ACTIVATE" ]; then + # shellcheck disable=SC1090 + source "$VENV_ACTIVATE" +else + echo "Error: venv not found at $VENV_ACTIVATE" >&2 + exit 1 +fi
4-6: Broaden CUDA PATH detection.-if [ -d "/usr/local/cuda-12.8/bin" ]; then - export PATH="/usr/local/cuda-12.8/bin:$PATH" +if [ -d "/usr/local/cuda-12.8/bin" ]; then + export PATH="/usr/local/cuda-12.8/bin:$PATH" +elif [ -d "/usr/local/cuda/bin" ]; then + export PATH="/usr/local/cuda/bin:$PATH" fi
12-15: Persist PID and make logging explicit.-date > log.txt +date > log.txt -python -m xtts_api_server --listen --lowvram &>> log.txt & +python -m xtts_api_server --listen --lowvram >> log.txt 2>&1 & +echo $! > xtts.pidconf.sh (4)
23-29: Make file discovery deterministic and constrained to top-level files.Limit depth, ensure regular files, and sort results for stable menus.
-mapfile -t files < <(find /home/dwemer/xtts-api-server/ -name "start-*.sh") +mapfile -t files < <(find /home/dwemer/xtts-api-server/ -maxdepth 1 -type f -name "start-*.sh" | sort)
33-35: Display basenames in the menu for readability; keep full path for linking.-for i in "${!files[@]}"; do - echo "$((i+1)). ${files[$i]}" -done +for i in "${!files[@]}"; do + echo "$((i+1)). $(basename -- "${files[$i]}")" +done @@ -echo "You selected: $selected_file" +echo "You selected: $(basename -- "$selected_file")"Also applies to: 59-61
1-1: Enable strict mode to catch errors early.#!/bin/bash +set -Eeuo pipefail
23-23: Avoid hardcoded absolute paths; derive repository root from script location.Improves portability across environments and usernames.
-mapfile -t files < <(find /home/dwemer/xtts-api-server/ -maxdepth 1 -type f -name "start-*.sh" | sort) +SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)" +REPO_DIR="$SCRIPT_DIR" +mapfile -t files < <(find "$REPO_DIR" -maxdepth 1 -type f -name "start-*.sh" | sort) @@ -ln -sf "$selected_file" "/home/dwemer/xtts-api-server/start.sh" +ln -sf "$selected_file" "$REPO_DIR/start.sh" @@ -chmod +x /home/dwemer/xtts-api-server/start-*.sh 2>/dev/null || true +chmod +x "$REPO_DIR"/start-*.sh 2>/dev/null || trueAlso applies to: 61-66
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
conf.sh(1 hunks)start-lowvram.sh(1 hunks)start-regular.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
conf.sh
[error] 20-20: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
start-lowvram.sh
[warning] 8-8: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
start-regular.sh
[warning] 8-8: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| # Display the files in a numbered list | ||
| echo -e "Select a an option from the list:\n\n" | ||
| for i in "${!files[@]}"; do | ||
| echo "$((i+1)). ${files[$i]}" | ||
| done | ||
|
|
||
| echo "0. Disable service"; | ||
| echo | ||
|
|
There was a problem hiding this comment.
Fix typos in user-facing text (“deepseed” → “deepspeed”; “Select a an” → “Select an”).
User-visible messages should be correct and professional.
-echo -e "Select a an option from the list:\n\n"
+echo -e "Select an option from the list:\n\n"
@@
-* deepseed = Uses more VRAM but faster
+* deepspeed = Uses more VRAM but faster📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Display the files in a numbered list | |
| echo -e "Select a an option from the list:\n\n" | |
| for i in "${!files[@]}"; do | |
| echo "$((i+1)). ${files[$i]}" | |
| done | |
| echo "0. Disable service"; | |
| echo | |
| # Display the files in a numbered list | |
| echo -e "Select an option from the list:\n\n" | |
| for i in "${!files[@]}"; do | |
| echo "$((i+1)). ${files[$i]}" | |
| done | |
| echo "0. Disable service"; | |
| echo |
🤖 Prompt for AI Agents
In conf.sh around lines 31 to 39, fix typos in user-facing strings: change
"Select a an option from the list:" to "Select an option from the list:" and
replace any occurrences of "deepseed" with the correct "deepspeed" in the file
so all displayed messages are spelled correctly and professional.
Moved cu124 to cu128
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (8)
ddistro_install.sh (8)
4-5: Parameterize hard-coded paths (BASE_DIR, VENV_DIR)
Make the script reusable across hosts and users.-python3 -m venv /home/dwemer/python-tts -source /home/dwemer/python-tts/bin/activate +BASE_DIR="${BASE_DIR:-$HOME/xtts-api-server}" +VENV_DIR="${VENV_DIR:-$HOME/python-tts}" +python3 -m venv "$VENV_DIR" +source "$VENV_DIR/bin/activate" @@ -source /home/dwemer/python-tts/bin/activate +source "$VENV_DIR/bin/activate"Also applies to: 49-49
1-1: Enable strict mode to fail fast
Add robust shell options right after the shebang.#!/bin/bash +set -euo pipefail +IFS=$'\n\t'
3-3: Check cd failure
Guard against running in the wrong directory.-cd /home/dwemer/xtts-api-server +cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to /home/dwemer/xtts-api-server"; exit 1; }
16-26: Consistently use the venv’s pip; avoid mixing pip invocations
Avoid ambiguity about interpreter and environment.- python3 -m pip install --upgrade pip wheel ninja virtualenv - pip install setuptools==68.1.2 + python3 -m pip install --upgrade pip wheel ninja virtualenv + python3 -m pip install setuptools==68.1.2 @@ - pip install --no-deps -r requirements.txt + python3 -m pip install --no-deps -r requirements.txt @@ - pip cache purge || true - pip uninstall -y torch torchaudio torchcodec torchvision || true - pip install --no-deps --no-cache-dir --index-url "$torch_url" "torch==${torch_ver}+${cu_tag}" "torchaudio==${torchaudio_ver}+${cu_tag}" - pip check || true + python3 -m pip cache purge || true + python3 -m pip uninstall -y torch torchaudio torchcodec torchvision || true + python3 -m pip install --no-deps --no-cache-dir --index-url "$torch_url" "torch==${torch_ver}+${cu_tag}" "torchaudio==${torchaudio_ver}+${cu_tag}" + python3 -m pip check || true @@ - python3 -m pip install --upgrade pip wheel ninja virtualenv - pip install setuptools==68.1.2 + python3 -m pip install --upgrade pip wheel ninja virtualenv + python3 -m pip install setuptools==68.1.2 @@ - pip install --no-deps -r requirements.txt + python3 -m pip install --no-deps -r requirements.txt @@ - pip cache purge || true - pip uninstall -y torch torchaudio torchcodec torchvision || true - pip install --index-url "$torch_url" torch torchaudio torchcodec torchvision - pip check || true + python3 -m pip cache purge || true + python3 -m pip uninstall -y torch torchaudio torchcodec torchvision || true + python3 -m pip install --index-url "$torch_url" torch torchaudio torchcodec torchvision + python3 -m pip check || true @@ - pip install --no-cache-dir soundfile + python3 -m pip install --no-cache-dir soundfileAlso applies to: 31-41
45-45: Avoid brittle sed patch and hard-coded Python 3.11 site-packages path
This will break across Python versions and TTS releases. Resolve site-packages dynamically and fail if the target pattern isn’t found.-sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py +SITE_PKGS="$("$VENV_DIR/bin/python" -c 'import site,sys; print(next(p for p in site.getsitepackages()+[site.getusersitepackages()] if p.endswith("site-packages")))')" +XTTS_PY="${SITE_PKGS}/TTS/tts/models/xtts.py" +if ! grep -q 'weights_only=False' "$XTTS_PY"; then + sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' "$XTTS_PY" +fiPrefer shipping a proper patch file or upstreaming the change.
47-47: Check sample exists; ensure speakers dir
Prevent confusing failures when the WAV is missing and ensure the target directory exists.-cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav +SAMPLE_PATH="${SAMPLE_PATH:-/home/dwemer/TheNarrator.wav}" +mkdir -p speakers +if [[ ! -f "$SAMPLE_PATH" ]]; then + echo "Error: sample file not found at $SAMPLE_PATH" >&2 + exit 1 +fi +cp "$SAMPLE_PATH" "speakers/TheNarrator.wav"
53-61: Fix contradictory prompts and start the server using the venv interpreter
The script asks users to wait for server output before actually starting it and uses system python.-echo -echo "This will start CHIM XTTS to download the selected model" -echo "Wait for the message 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'" -echo "Then close this window. Press ENTER to continue" -read - -echo "please wait...." - -python -m xtts_api_server --listen +echo +echo "Press ENTER to start CHIM XTTS and download the selected model." +echo "After startup, look for: 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'." +read -r +echo "Starting server, please wait..." +"$VENV_DIR/bin/python" -m xtts_api_server --listenConsider documenting how to keep it running (systemd/tmux/screen) instead of “close this window”.
61-61: Ensure xtts_api_server is installed/importable before invoking
Install the local package (editable or regular) so the module is available.-"$VENV_DIR/bin/python" -m xtts_api_server --listen +# Ensure local package is installed (adjust if packaged differently) +"$VENV_DIR/bin/python" -m pip install -e . +"$VENV_DIR/bin/python" -m xtts_api_server --listen
🧹 Nitpick comments (5)
ddistro_install.sh (5)
10-10: Use read -r to avoid backslash escapes
Small robustness improvement for user prompts.-read -p "Are you using a GT10XX series GPU? (yes/no): " gpu_answer +read -r -p "Are you using a GT10XX series GPU? (yes/no): " gpu_answer @@ -read +read -rAlso applies to: 57-57
28-39: CUDA index choice looks OK; consider pinning versions for cu128 too
cu128 wheels exist for recent PyTorch (e.g., 2.7.1 GA/test, nightly 2.8), so the index is valid. Still pin torch/torchaudio versions for reproducibility like you did for cu118.You can set TORCH_VER/TORCHAUDIO_VER defaults and allow overrides:
- cu_tag="cu128" + cu_tag="cu128" + torch_ver="${TORCH_VER:-2.7.1}" + torchaudio_ver="${TORCHAUDIO_VER:-2.7.1}" @@ - python3 -m pip install --index-url "$torch_url" torch torchaudio torchcodec torchvision + python3 -m pip install --no-deps --no-cache-dir --index-url "$torch_url" "torch==${torch_ver}+${cu_tag}" "torchaudio==${torchaudio_ver}+${cu_tag}" + python3 -m pip install --no-cache-dir --index-url "$torch_url" torchvision torchcodecCitations: PyTorch announces cu128 wheels for 2.7.1 and shows nightly cu128 wheels. (dev-discuss.pytorch.org, github.com, huggingface.co)
51-51: Validate conf.sh before executing
Fail fast if missing or not executable.-./conf.sh +if [[ ! -x ./conf.sh ]]; then + echo "Error: conf.sh not found or not executable" >&2 + exit 1 +fi +./conf.sh
63-63: Stray prompt
Either remove it or actually read input.-echo "Press Enter" +# echo "Press Enter" +# read -r
21-24: Don’t mask dependency issues with|| true
Let pip failures fail the script in strict mode.- python3 -m pip cache purge || true + python3 -m pip cache purge @@ - python3 -m pip check || true + python3 -m pip check @@ - python3 -m pip cache purge || true + python3 -m pip cache purge @@ - python3 -m pip check || true + python3 -m pip checkAlso applies to: 36-39
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
conf.sh (4)
10-14: Correct user-facing typos (“deepseed” → “deepspeed”; “Select a an” → “Select an”).Options: -* deepseed = Uses more VRAM but it is faster. Does not work with 50xx series GPUs. +* deepspeed = Uses more VRAM but it is faster. Does not work with 50xx series GPUs. @@ -echo -e "Select a an option from the list:\n\n" +echo -e "Select an option from the list:\n\n"Also applies to: 31-33
61-61: Quote path variables in ln -sf.Prevents issues with spaces or special chars.
-ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh +ln -sf "$selected_file" "/home/dwemer/xtts-api-server/start.sh"
19-21: Fix invalid use of exit with a string.Use echo to print the error, then exit with a non-zero code.
if [ ! -d /home/dwemer/python-tts ]; then - exit "XTTSv2 not installed" + echo "Error: XTTSv2 not installed" >&2 + exit 1 fi
45-54: Reorder/adjust validation to avoid integer-expression errors.Using -eq before confirming numeric can crash on non-numeric input.
-if [ "$selection" -eq "0" ]; then +if [ "$selection" = "0" ]; then echo "Disabling service. Run this again to enable" rm /home/dwemer/xtts-api-server/start.sh &>/dev/null exit 0 fi -if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then +if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt "${#files[@]}" ]; then echo "Invalid selection." exit 1 fi
🧹 Nitpick comments (6)
conf.sh (6)
47-47: Also quote the path in rm.- rm /home/dwemer/xtts-api-server/start.sh &>/dev/null + rm "/home/dwemer/xtts-api-server/start.sh" &>/dev/null
41-41: Use read -r to avoid backslash escapes.-read -p "Select an option by picking the matching number: " selection +read -r -p "Select an option by picking the matching number: " selection
23-23: Deterministic, safer file discovery.Quote the path, restrict depth/type, and sort for stable menus.
-mapfile -t files < <(find /home/dwemer/xtts-api-server/ -name "start-*.sh") +mapfile -t files < <(find "/home/dwemer/xtts-api-server/" -maxdepth 1 -type f -name "start-*.sh" | sort)
6-8: Clarify message: this script configures, not installs.Avoid confusing users about its purpose.
-This will install CHIM XTTS. This is a high quality TTS service that works with Skyrim voices. +This configures CHIM XTTS start mode. CHIM XTTS is a high quality TTS service that works with Skyrim voices.
64-65: Ensure executability before linking (order).Make start-*.sh executable before creating/refreshing the symlink.
-# Ensure all start scripts are executable -chmod +x /home/dwemer/xtts-api-server/start-*.sh 2>/dev/null || true +# Ensure all start scripts are executable before linking +chmod +x /home/dwemer/xtts-api-server/start-*.sh 2>/dev/null || trueConsider moving these two lines above the ln -sf step.
1-1: Harden the script.Enable strict mode for safer failures.
#!/bin/bash +set -Eeuo pipefail +IFS=$'\n\t'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
conf.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
conf.sh
[error] 20-20: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
Updated PyTorch installation to specific versions with CUDA support.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
ddistro_install.sh (3)
3-3: Handlecdfailure to avoid running in the wrong directory.If the
cdon Line [3] fails (typo, permissions, missing repo), the rest of the script keeps running and starts installing into whatever directory the caller happened to be in. Add an explicit guard so we fail fast.- cd /home/dwemer/xtts-api-server + cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to /home/dwemer/xtts-api-server"; exit 1; }
48-48: Check the sample file exists before copying.If
/home/dwemer/TheNarrator.wavis missing, thecpon Line [48] prints an error and the script keeps going, leaving the install half-baked. Guard it:-cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav +sample_src=/home/dwemer/TheNarrator.wav +if [[ ! -f "$sample_src" ]]; then + echo "Error: sample file not found at $sample_src" + exit 1 +fi +cp "$sample_src" speakers/TheNarrator.wav
54-62: Fix the contradictory startup prompts.Lines [55-62] still tell users to “wait for” Uvicorn output and “close this window” before the server even starts, yet the server only launches after the
read. This blocks automation, confuses users, and the “close this window” instruction kills the server outright. Rework the flow so pressing ENTER starts the server immediately and drop the advice to close the terminal:-echo "This will start CHIM XTTS to download the selected model" -echo "Wait for the message 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'" -echo "Then close this window. Press ENTER to continue" -read - -echo "please wait...." - -python -m xtts_api_server --listen - -echo "Press Enter" +echo "Press ENTER to start CHIM XTTS and download the selected model." +echo "Watch for: 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'." +echo "Leave this terminal open (or run under tmux/screen/systemd) to keep the server alive." +read -r +echo "Starting server, please wait..." +python -m xtts_api_server --listen
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
ddistro_install.sh
Outdated
| #pip install xtts-api-server #Fails | ||
| fi | ||
|
|
||
| sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py |
There was a problem hiding this comment.
Stop hard-coding the Python 3.11 site-packages path.
Line [46] assumes the venv lives under .../python3.11/..., but python3 -m venv will follow whatever interpreter version is installed (3.10, 3.12, etc.). On anything but 3.11 the sed command fails, the patch is never applied, and the setup is silently broken. Resolve the path at runtime:
-sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py
+model_file="$(
+ python - <<'PY'
+import sysconfig, pathlib
+print(pathlib.Path(sysconfig.get_path("purelib")) / "TTS/tts/models/xtts.py")
+PY
+)"
+if [[ ! -f "$model_file" ]]; then
+ echo "Error: xtts.py not found at $model_file"
+ exit 1
+fi
+sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' "$model_file"🤖 Prompt for AI Agents
In ddistro_install.sh around line 46, the sed command hard-codes a python3.11
site-packages path which will fail for other interpreter versions; change it to
resolve the site-packages directory at runtime by invoking the target Python
interpreter to print its site-packages/purelib path and use that value in the
sed command so the replacement operates on the correct venv path (ensure the
python invocation matches the venv/interpreter you created and properly
quote/escape the resolved path when passing it to sed).
Updated CUDA version handling and installation process for PyTorch and related packages. Added user prompt for CUDA version selection and modified installation commands accordingly.
Added a comprehensive list of dependencies for the project.
Add LD_LIBRARY_PATH for NVIDIA libraries
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
start-regular.sh (1)
8-10: Add error handling to thecdcommand (unresolved from previous review).The directory change lacks error handling. If the directory doesn't exist, the script will silently continue and execute subsequent commands in the wrong context, leading to activation of the venv from an unexpected location.
Apply this diff to add error handling:
-cd /home/dwemer/xtts-api-server/ +cd /home/dwemer/xtts-api-server/ || { echo "Error: failed to change directory"; exit 1; }
🧹 Nitpick comments (1)
start-regular.sh (1)
4-5: Consider making CUDA version configurable.The CUDA version (12.8) is hardcoded. This may cause issues if the installed CUDA version differs. Consider allowing this to be overridden via an environment variable or auto-detected.
-if [ -d "/usr/local/cuda-12.8/bin" ]; then - export PATH="/usr/local/cuda-12.8/bin:$PATH" +CUDA_VERSION="${CUDA_VERSION:-12.8}" +CUDA_PATH="${CUDA_PATH:-/usr/local/cuda-${CUDA_VERSION}}" +if [ -d "${CUDA_PATH}/bin" ]; then + export PATH="${CUDA_PATH}/bin:$PATH"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
start-regular.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
start-regular.sh
[warning] 8-8: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (1)
start-regular.sh (1)
8-8: Partial review comment is incorrect — line 12 does not contain the hardcoded nvidia path cited.The hardcoded paths at lines 8 and 10 in start-regular.sh are correctly identified and should be made portable. However, the review incorrectly claims line 12 contains
/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/— line 12 isdate > log.txt, which is not a hardcoded path.The nvidia CUDA library path does exist in other scripts (start-lowvram.sh:14, start-deepspeed.sh:10, ddistro_install.sh:83), but not in start-regular.sh. The "Also applies to: 10-10, 12-12" statement is partially incorrect.
Lines 8 and 10 warrant fixes; line 12 does not apply to this script.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
requirements5.txt (1)
1-177: Document the purpose and scope ofrequirements5.txtrelative to other requirements files.The file contains 177 strictly pinned dependencies spanning ML/NLP/TTS ecosystems (transformers, spacy, coqui-tts, stanza, gruut, torch + CUDA stack, etc.). This appears to be a complete environment setup, not a minimal set for an upload API route.
Please clarify:
- What is the rationale for creating a separate
requirements5.txt?- Is this intended to replace or supplement existing
requirements.txt?- Are all 177 dependencies necessary for the
/upload_samplefeature, or is this a broader environment manifest?- Why include extensive ML tooling (e.g., spacy, coqui-tts-trainer, deepspeed) for a file upload endpoint?
Consider adding a comment block at the top of the file documenting its purpose, deployment target, and relationship to other requirements files in the repository.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
requirements5.txt(1 hunks)
🔇 Additional comments (3)
requirements5.txt (3)
1-177: Mismatch between PR objectives and file content—verify scope.The PR objectives describe adding a
/upload_sampleroute to the Web API, but the file under review is a comprehensive requirements manifest containing 177 dependencies. The AI summary differs significantly from the PR objectives. Please clarify:
- Are the main code changes (the
/upload_sampleroute handler) included in a separate file not shown here?- Is
requirements5.txtintentionally separate fromrequirements.txt, and what is its purpose?- Why does a new upload route warrant such an extensive dependency set?
126-126: Now I'll verify the compatibility between these versions:Verify version compatibility: all three packages are compatible and current.
FastAPI requires python-multipart >=0.0.18. python-multipart 0.0.20 was released Dec 16, 2024 and satisfies this requirement. All three versions—python-multipart 0.0.20, FastAPI 0.115.6, and uvicorn 0.34.0—are recent stable releases from December 2024 with no known compatibility issues between them.
172-175: Based on the verification, the original review comment contains a factual error. Local version identifiers are PEP 440 compliant and follow the scheme:<public version identifier>[+<local version label>]. The+cu130notation is therefore not "non-PEP 440 versioning" as claimed.However, the underlying concern about installation practices is worth addressing. PyPI does not allow uploading packages with local version identifiers, and while PyTorch publishes wheels with this notation, the official guidance (from the earlier web search) indicates that using
--index-url https://download.pytorch.org/whl/cu130is the primary recommended approach rather than pinning+cu130suffixes in requirements files.
Reconsider pinning strategy for CUDA-specific PyTorch wheels.
The
+cu130suffix is PEP 440-compliant local version notation. However, PyTorch's official installation guidance recommends using the--index-urlflag (e.g.,https://download.pytorch.org/whl/cu130) rather than pinning CUDA variants in requirements.txt. This ensures better reproducibility and compatibility.Consider:
- Adding
--index-url https://download.pytorch.org/whl/cu130to your pip install command, or- Documenting the exact pip command needed to install these requirements with the correct PyTorch index
Conditionally add CUDA paths and libraries if the directory exists.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
start-deepspeed.sh (2)
7-7: Replace hardcoded paths with script-relative or environment-aware paths.Absolute paths like
/home/dwemer/...are not portable across development, test, and production environments. Use relative paths or environment variables to make this script reusable.Consider parameterizing critical paths:
+#!/bin/bash +# Configuration +PROJECT_DIR="${PROJECT_DIR:-$(dirname "$0")}" +VENV_PATH="${VENV_PATH:-${PROJECT_DIR}/../python-tts}" +CUDA_PATH="${CUDA_PATH:-/usr/local/cuda-12.8}" -if [ -d "/usr/local/cuda-12.8/bin" ]; then - export PATH="/usr/local/cuda-12.8/bin:$PATH" +if [ -d "${CUDA_PATH}/bin" ]; then + export PATH="${CUDA_PATH}/bin:$PATH" fi -cd /home/dwemer/xtts-api-server/ || exit 1 +cd "${PROJECT_DIR}" || { echo "Error: Failed to change directory to ${PROJECT_DIR}"; exit 1; } -source /home/dwemer/python-tts/bin/activate || exit 1 +source "${VENV_PATH}/bin/activate" || { echo "Error: Failed to activate venv at ${VENV_PATH}"; exit 1; } -if [ -d "/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/" ]; +if [ -d "${VENV_PATH}/lib/python3.11/site-packages/nvidia/cu13/lib/" ]; then - export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/ - export PATH=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/bin:$PATH - export CUDA_HOME=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13 + export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${VENV_PATH}/lib/python3.11/site-packages/nvidia/cu13/lib/" + export PATH="${VENV_PATH}/lib/python3.11/site-packages/nvidia/cu13/bin:${PATH}" + export CUDA_HOME="${VENV_PATH}/lib/python3.11/site-packages/nvidia/cu13" fiThis approach allows users to override paths via environment variables or symlinks without editing the script.
Also applies to: 9-9, 14-14, 15-15, 16-16
19-19: Consider capturing process PID when launching background task.Running the server with
&creates a background process, but the PID is not captured. This can make graceful shutdown or health monitoring difficult.Consider storing the PID for lifecycle management:
-python -m xtts_api_server --deepspeed --listen &> log.txt & +python -m xtts_api_server --deepspeed --listen &> log.txt & +echo $! > .xtts_server.pidThis allows other scripts to query or terminate the server by reading the PID from
.xtts_server.pid.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
requirements5.txt(1 hunks)start-deepspeed.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
start-deepspeed.sh
[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| nvidia-cuda-nvcc | ||
| nvidia-cuda-profiler-api-cu13 | ||
| nvidia-cuda-profiler-api | ||
|
|
There was a problem hiding this comment.
Fix incomplete package specifications.
Lines 177–179 lack version constraints, and line 180 contains malformed data. These prevent reproducible builds and may cause installation failures.
Apply this diff to fix the incomplete entries:
-nvidia-cuda-nvcc
-nvidia-cuda-profiler-api-cu13
-nvidia-cuda-profiler-api
-+
+nvidia-cuda-nvcc==13.0.39
+nvidia-cuda-profiler-api-cu13==0.0.0a0
+nvidia-cuda-profiler-api==13.0.39If the intent is to omit these packages, remove them entirely. If they should truly have no version constraint, consider documenting why.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In requirements5.txt around lines 177 to 180, the three CUDA-related entries are
incomplete (no version constraints) and one line is malformed; update these
lines by either removing the packages entirely if they are not required, or
pinning them to explicit, tested versions (e.g., nvidia-cuda-nvcc==<version>,
nvidia-cuda-profiler-api-cu13==<version>, nvidia-cuda-profiler-api==<version>)
and remove the malformed line; if you intentionally want no pinning, leave them
out of this requirements file and add a comment explaining why unpinned CUDA
packages are acceptable for reproducibility.
| export PATH="/usr/local/cuda-12.8/bin:$PATH" | ||
| fi | ||
|
|
||
| cd /home/dwemer/xtts-api-server/ |
There was a problem hiding this comment.
Add error handling to cd and source commands.
Both commands can fail silently, allowing subsequent lines to execute in an unexpected state. This is a critical failure mode.
Apply this diff to add error handling:
-cd /home/dwemer/xtts-api-server/
+cd /home/dwemer/xtts-api-server/ || { echo "Error: Failed to change directory"; exit 1; }
-source /home/dwemer/python-tts/bin/activate
+source /home/dwemer/python-tts/bin/activate || { echo "Error: Failed to activate venv"; exit 1; }Also applies to: 9-9
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
start-deepspeed.sh lines 7 and 9: the cd and source commands can fail silently;
either enable strict shell failure handling at the top of the script (e.g., set
-euo pipefail) or add explicit checks immediately after each cd and source
invocation that detect failure, print a clear error message to stderr, and exit
with a non-zero status so the script stops on error.
Added symlink creation for CUDA libraries and updated environment variables for CUDA paths.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
ddistro_install.sh (6)
16-76: Consistently usepython3 -m pipinstead of barepipwithin the venv.This issue was flagged in a previous review. Mixing bare
pipandpython3 -m pipcan lead to ambiguity about which interpreter is used.Replace all bare
pipinvocations withpython3 -m pipthroughout the script (lines 17, 19, 22–23, 36, 39, 61, 63, 66, 68, etc.). Example:-pip install setuptools==68.1.2 +python3 -m pip install setuptools==68.1.2 -pip install --no-deps -r requirements.txt +python3 -m pip install --no-deps -r requirements.txt
79-79: Check file existence before copying speaker sample.This issue was flagged in a previous review. Copying without verifying the source file exists will fail with an unclear error.
+if [[ ! -f "/home/dwemer/TheNarrator.wav" ]]; then + echo "Error: sample file not found at /home/dwemer/TheNarrator.wav" + exit 1 +fi cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav
3-3: Wrapcdwith error handling.This issue was flagged in a previous review but remains unaddressed. If the directory change fails, subsequent commands run in the wrong context.
-cd /home/dwemer/xtts-api-server +cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to xtts-api-server"; exit 1; }
27-27: Resolve site-packages path dynamically instead of hard-coding Python 3.11.This issue was flagged in a previous review. Hard-coding
python3.11will fail silently on any other interpreter version (3.10, 3.12+). Notably, line 45 already demonstrates the correct approach usingpython3 -c 'import site; ...', but lines 27 and 73 still hard-code the path.Apply this dynamic resolution to both line 27 and line 73:
+model_file="$( + python3 - <<'PY' +import sysconfig +print(sysconfig.get_path("purelib") + "/TTS/tts/models/xtts.py") +PY +)" +if [[ ! -f "$model_file" ]]; then + echo "Error: xtts.py not found at $model_file" + exit 1 +fi -sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py +sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' "$model_file"Repeat this pattern for both occurrences (line 27 in the cu118 branch and line 73 in the cu128 branch).
Also applies to: 73-73
94-99: Resolve CUDA library paths dynamically instead of hard-coding Python 3.11.This issue relates to the hard-coded site-packages path flagged elsewhere (lines 27, 73). Hard-coding
python3.11will fail silently on other interpreter versions.-if [ -d "/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/" ]; +cu13_lib_dir="$(python3 -c 'import sysconfig; print(sysconfig.get_path("purelib") + "/nvidia/cu13/lib")' 2>/dev/null)" +if [ -n "$cu13_lib_dir" ] && [ -d "$cu13_lib_dir" ]; then - export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/ - export PATH=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/bin:$PATH - export CUDA_HOME=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13 + export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$cu13_lib_dir + export PATH=$(python3 -c 'import sysconfig; print(sysconfig.get_path("purelib") + "/nvidia/cu13/bin")'):$PATH + export CUDA_HOME=$(python3 -c 'import sysconfig; print(sysconfig.get_path("purelib") + "/nvidia/cu13")') fi
103-103: Usepython3(or the venv's Python) instead of barepython.This issue was flagged in a previous review. Using bare
pythonmay bypass the venv or use an incorrect interpreter.-python -m xtts_api_server --listen $DEEPSPEED +python3 -m xtts_api_server --listen $DEEPSPEEDAlternatively, use the venv's Python directly for stronger isolation:
"${VENV_DIR:-/home/dwemer/python-tts}/bin/python" -m xtts_api_server --listen $DEEPSPEED
🧹 Nitpick comments (3)
ddistro_install.sh (3)
89-89: Useread -rto prevent backslash interpretation.The
readcommand without-rwill interpret backslashes as escape sequences, potentially causing unexpected behavior when users input certain characters.-read +read -r
101-101: Add defensive check forstart.shsymlink.The
readlinkcommand will fail silently ifstart.shdoesn't exist or isn't a symlink, andDEEPSPEEDwill be empty. Consider adding a check to confirm the file exists and provide clearer intent.-readlink start.sh | grep -q '/home/dwemer/xtts-api-server/start-deepspeed.sh' && export DEEPSPEED="--deepspeed" || export DEEPSPEED="" +if [ -L start.sh ]; then + readlink start.sh | grep -q 'start-deepspeed.sh' && export DEEPSPEED="--deepspeed" || export DEEPSPEED="" +else + export DEEPSPEED="" +fi
54-54: Remove or clarify commented-out code.Lines 54 and 72 contain commented
#pip install xtts-api-server #Failswith no explanation. These should either be removed or replaced with a clear TODO/FIXME explaining why installation fails and what the workaround is.- #pip install xtts-api-server #Fails + # TODO: xtts-api-server install fails; investigate dependency issue or install via editable mode (pip install -e .)Alternatively, remove these lines entirely if they're no longer relevant.
Also applies to: 72-72
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
Add cleanup step for previous deepspeed files.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
ddistro_install.sh (5)
3-3: Guard thecdcommand with error handling.If the directory change fails, subsequent commands will execute in the wrong context, corrupting the environment setup. Previously flagged; still unresolved.
-cd /home/dwemer/xtts-api-server +cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to xtts-api-server"; exit 1; }
87-89: Fix contradictory user messaging and flow.The prompt tells the user to "wait for Uvicorn message" before starting the server, and then instructs them to "close this window"—which would kill the running process. This is confusing and incorrect. Clarify the sequence: the script will start the server immediately after the user presses Enter.
echo -echo "This will start CHIM XTTS to download the selected model" -echo "Wait for the message 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'" -echo "Then close this window. Press ENTER to continue" -read +echo "Press ENTER to start CHIM XTTS and download the selected model." +echo "After startup, look for: 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'." +read -r
1-2: Add strict error handling after shebang.The script lacks
set -euo pipefailfor proper error propagation and safe defaults (exit on error, undefined variables, pipe failures). This allows the script to silently continue when commands fail.#!/bin/bash +set -euo pipefail cd /home/dwemer/xtts-api-server
80-80: Verify speaker file exists before copying; parameterize paths.The copy operation has no existence check and hard-codes the sample path, making it fragile. If the file is missing or in a different location, the command silently fails or produces a cryptic error. Previously flagged; still unresolved.
+SAMPLE_FILE="/home/dwemer/TheNarrator.wav" +if [[ ! -f "$SAMPLE_FILE" ]]; then + echo "Error: sample file not found at $SAMPLE_FILE" >&2 + exit 1 +fi -cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav +cp "$SAMPLE_FILE" speakers/TheNarrator.wav
28-28: Resolve the site-packages path at runtime instead of hard-coding python3.11.The
sedpath assumespython3.11butpython3 -m venvadapts to the installed Python version (3.10, 3.12, etc.). On any version other than 3.11, this command silently fails, leaving the patch unapplied. Previously flagged; still unresolved. Use the venv's interpreter to resolve the path dynamically.- sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py + model_file="$(/home/dwemer/python-tts/bin/python -c 'import sysconfig, pathlib; print(pathlib.Path(sysconfig.get_path("purelib")) / "TTS/tts/models/xtts.py")')" + if [[ ! -f "$model_file" ]]; then + echo "Error: xtts.py not found at $model_file" >&2 + exit 1 + fi + sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' "$model_file"
🧹 Nitpick comments (4)
ddistro_install.sh (4)
18-27: Consistently usepython3 -m pipfor clarity and robustness.Line 17 correctly uses
python3 -m pip, but subsequent lines revert to barepip. This inconsistency can lead to ambiguity about which interpreter is invoked. Standardize onpython3 -m pipthroughout.- pip install setuptools==68.1.2 - # Install app requirements without auto-pulling torch/torchaudio from deps - pip install --no-deps -r requirements.txt + python3 -m pip install setuptools==68.1.2 + # Install app requirements without auto-pulling torch/torchaudio from deps + python3 -m pip install --no-deps -r requirements.txt # Pin to stable, CUDA-tagged PyTorch/Torchaudio that do not require TorchCodec pip cache purge || true - pip uninstall -y torch torchaudio torchcodec torchvision || true - pip install --no-deps --no-cache-dir --index-url "$torch_url" "torch==${torch_ver}+${cu_tag}" "torchaudio==${torchaudio_ver}+${cu_tag}" - pip check || true + python3 -m pip uninstall -y torch torchaudio torchcodec torchvision || true + python3 -m pip install --no-deps --no-cache-dir --index-url "$torch_url" "torch==${torch_ver}+${cu_tag}" "torchaudio==${torchaudio_ver}+${cu_tag}" + python3 -m pip check || true # Ensure fallback audio loader is available - pip install --no-cache-dir soundfile + python3 -m pip install --no-cache-dir soundfile
37-44: Standardize onpython3 -m pipfor consistency across all CUDA paths.Both the CUDA 13.0 and CUDA 12.8 branches use bare
pipinconsistently. Line 46 already demonstrates the correct pattern for dynamic resolution; apply the same discipline to all pip invocations.Also applies to: 62-72
102-102: Resolve the start script symlink target using the venv's Python and parameterized paths.This grep command hard-codes
/home/dwemer/xtts-api-server/start-deepspeed.sh, making it brittle to path changes. Additionally,readlinkoutput may be a relative symlink; use the venv's Python to compute it reliably.-readlink start.sh | grep -q '/home/dwemer/xtts-api-server/start-deepspeed.sh' && export DEEPSPEED="--deepspeed" || export DEEPSPEED="" +SYMLINK_TARGET="$(readlink -f start.sh)" || SYMLINK_TARGET="" +[[ "$SYMLINK_TARGET" == *"start-deepspeed.sh"* ]] && export DEEPSPEED="--deepspeed" || export DEEPSPEED=""
3-5: Parameterize base directory and venv paths to improve portability and reusability.Hard-coding
/home/dwemerpaths makes this script specific to one user and environment. Allow overrides via environment variables with sensible defaults so users can adapt it to their setup without editing the script.Add near the top (after shebang and
set -euo pipefail):# Allow users to override paths via environment variables BASE_DIR="${BASE_DIR:-$HOME/xtts-api-server}" VENV_DIR="${VENV_DIR:-$HOME/python-tts}" SAMPLE_FILE="${SAMPLE_FILE:-$HOME/TheNarrator.wav}" cd "$BASE_DIR" || { echo "Error: cannot cd to $BASE_DIR"; exit 1; } python3 -m venv "$VENV_DIR" source "$VENV_DIR/bin/activate"This allows users to run:
BASE_DIR=/opt/xtts VENV_DIR=/opt/venv ./ddistro_install.sh
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| # Ensure fallback audio loader is available | ||
| pip install --no-cache-dir soundfile | ||
| #pip install xtts-api-server #Fails | ||
| sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py |
There was a problem hiding this comment.
Resolve python version path dynamically (same as line 28 issue).
Line 74 repeats the hard-coded python3.11 path problem. Apply the same fix used for line 28: compute the site-packages path at runtime.
- sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py
+ model_file="$(/home/dwemer/python-tts/bin/python -c 'import sysconfig, pathlib; print(pathlib.Path(sysconfig.get_path("purelib")) / "TTS/tts/models/xtts.py")')"
+ if [[ ! -f "$model_file" ]]; then
+ echo "Error: xtts.py not found at $model_file" >&2
+ exit 1
+ fi
+ sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' "$model_file"🤖 Prompt for AI Agents
In ddistro_install.sh around line 74, the sed invocation uses a hard-coded
/home/dwemer/python3.11/.../site-packages path; compute the site-packages path
at runtime (as done at line 28) and use that variable in the sed command instead
of the literal path. Set a SITE_PKG variable by invoking the chosen python
interpreter to get site-packages (matching the interpreter used earlier), then
run sed against "$SITE_PKG/.../TTS/tts/models/xtts.py" so the script works
across Python versions and installs.
| if [ -d "/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/" ]; | ||
| then | ||
| export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/ | ||
| export PATH=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/bin:$PATH | ||
| export CUDA_HOME=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13 | ||
| fi |
There was a problem hiding this comment.
Use runtime path resolution for CUDA libraries instead of hard-coded python3.11.
The conditional path and environment variables hard-code python3.11. This will fail silently on other Python versions.
-if [ -d "/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/" ];
+CUDA_LIB_PATH="$(/home/dwemer/python-tts/bin/python -c 'import sysconfig, pathlib; p = pathlib.Path(sysconfig.get_path("purelib")) / "nvidia/cu13/lib"; print(p if p.exists() else "")')"
+if [ -n "$CUDA_LIB_PATH" ] && [ -d "$CUDA_LIB_PATH" ];
then
- export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/
- export PATH=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/bin:$PATH
- export CUDA_HOME=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13
+ export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${CUDA_LIB_PATH}"
+ export PATH="$(dirname "$CUDA_LIB_PATH")/bin:$PATH"
+ export CUDA_HOME="$(dirname "$CUDA_LIB_PATH")/.."
fiCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ddistro_install.sh around lines 95-100, the script hard-codes a python3.11
site-packages path for CUDA libs; replace the hard-coded path with a
runtime-resolved site-packages base (call python to print the site-packages or
sysconfig path and store it in a variable) and then build the nvidia/cu13 lib
and bin paths from that variable before checking/setting LD_LIBRARY_PATH, PATH
and CUDA_HOME so the script works for other Python versions/environments.
|
|
||
| readlink start.sh | grep -q '/home/dwemer/xtts-api-server/start-deepspeed.sh' && export DEEPSPEED="--deepspeed" || export DEEPSPEED="" | ||
|
|
||
| python -m xtts_api_server --listen $DEEPSPEED |
There was a problem hiding this comment.
Install the local package and use the venv's Python explicitly.
Line 104 attempts to run xtts_api_server without installing it first, and uses bare python instead of python3. The module is not importable unless installed (e.g., pip install -e . or pip install .). This is a critical blocker; previously flagged, still unresolved.
Add a local install step before this line (around line 84, after conf.sh):
# Install the local xtts_api_server package
python3 -m pip install -e . || python3 -m pip install .Then update line 104:
-python -m xtts_api_server --listen $DEEPSPEED
+/home/dwemer/python-tts/bin/python -m xtts_api_server --listen $DEEPSPEEDAlternatively, add a console-script entrypoint in pyproject.toml so pip install . creates an executable:
[project.scripts]
xtts-api-server = "xtts_api_server.__main__:main"Then invoke it directly:
xtts-api-server --listen $DEEPSPEED🤖 Prompt for AI Agents
In ddistro_install.sh around lines 84 and 104: the script runs the module with a
bare `python` at line 104 without installing the package, so it won't be
importable; add a local install step after sourcing conf.sh (around line 84)
that runs the venv's pip via python3 (e.g., python3 -m pip install -e . ||
python3 -m pip install .) to install the package, and change the invocation on
line 104 to use the venv's python3 explicitly (e.g., python3 -m xtts_api_server
--listen $DEEPSPEED) or, alternatively, add a console-script entrypoint in
pyproject.toml so `pip install .` creates an executable and call that executable
instead.
3 participants
Summary by CodeRabbit
New Features
Chores