fix: install Runtipi in /opt instead of default location#19
Conversation
📝 WalkthroughWalkthroughLe script d'installation système ChangesCorrections du script d'installation système
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stage-tipi/01-config/files/app/setup.py (1)
384-397:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRisque de fuite de processus si le
cwd="/opt"échoue (ordre de lancement).À la Line 384,
curlest lancé avant la création du processbash(Line 389). SiPopen(..., cwd="/opt")échoue à la Line 396 (répertoire absent/inaccessible),curln’est ni terminé ni attendu dans ce chemin d’erreur, ce qui peut laisser des processus orphelins pendant les retries.💡 Correctif proposé
def install_runtipi(max_attempts: int = 3) -> bool: step(T["runtipi_step"]) for attempt in range(1, max_attempts + 1): if attempt > 1: out(T["runtipi_retry"].format(attempt=attempt, total=max_attempts)) time.sleep(30) + curl = None try: + if not os.path.isdir("/opt"): + err(T["runtipi_err"].format(e="Le répertoire /opt est introuvable")) + continue curl = subprocess.Popen( ["curl", "-L", "--max-time", "120", "https://setup.runtipi.io"], stdout=subprocess.PIPE, stderr=subprocess.PIPE, ) bash = subprocess.Popen( ["bash"], stdin=curl.stdout, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, bufsize=1, cwd="/opt", ) curl.stdout.close() @@ except Exception as e: err(T["runtipi_err"].format(e=e)) + if curl and curl.poll() is None: + curl.terminate() + curl.wait(timeout=5)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stage-tipi/01-config/files/app/setup.py` around lines 384 - 397, The current sequence launches curl then attempts to spawn bash with cwd="/opt", which can raise and leave curl orphaned; wrap the creation of the bash Popen in a try/except so that if Popen(["bash"], ..., cwd="/opt") fails you immediately terminate and wait on the previously-started curl (call curl.kill() and curl.wait()), or alternatively pre-validate/create/access-check the cwd (os.path.isdir and permissions) before starting either process; reference the variables curl and bash and the Popen calls so the fix ensures no orphaned curl processes remain on errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@stage-tipi/01-config/files/app/setup.py`:
- Around line 384-397: The current sequence launches curl then attempts to spawn
bash with cwd="/opt", which can raise and leave curl orphaned; wrap the creation
of the bash Popen in a try/except so that if Popen(["bash"], ..., cwd="/opt")
fails you immediately terminate and wait on the previously-started curl (call
curl.kill() and curl.wait()), or alternatively pre-validate/create/access-check
the cwd (os.path.isdir and permissions) before starting either process;
reference the variables curl and bash and the Popen calls so the fix ensures no
orphaned curl processes remain on errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6cf30413-23da-4cdf-befe-4a4d717cec9e
📒 Files selected for processing (1)
stage-tipi/01-config/files/app/setup.py
Context
Runtipi was being installed in the default directory used by its install
script. This PR explicitly sets
/optas the working directory, which isthe standard Linux convention for third-party software.
Changes
cwd="/opt"on thePopencall that runs the Runtipi install scriptSummary by CodeRabbit
Corrections de bugs
Chores