-
Notifications
You must be signed in to change notification settings - Fork 582
Use Conda for backend & sync-microservice setup on Windows #1076
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Use Conda for backend & sync-microservice setup on Windows #1076
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe Windows setup script ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SetupPS as "scripts/setup.ps1"
participant Miniconda
participant Conda
participant BackendEnv as backend_env
participant SyncEnv as sync_env
User->>SetupPS: run setup.ps1
SetupPS->>Conda: check for conda in PATH
alt conda not found
SetupPS->>Miniconda: download installer
Miniconda-->>SetupPS: install complete
SetupPS->>Conda: initialize conda for PowerShell (conda-hook / PATH)
end
SetupPS->>Conda: conda create -n backend_env python=3.12
Conda-->>BackendEnv: backend_env created
SetupPS->>Conda: conda run -n backend_env pip install -r backend/requirements.txt
SetupPS->>Conda: conda create -n sync_env python=3.12
Conda-->>SyncEnv: sync_env created
SetupPS->>Conda: conda run -n sync_env pip install -r sync-microservice/requirements.txt
SetupPS->>User: finish (frontend setup unchanged)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@scripts/setup.ps1`:
- Around line 169-176: The conda commands may fail silently in a non-interactive
PowerShell script; after running "conda create -n backend_env python=3.12 -y"
and "conda activate backend_env" check $LASTEXITCODE and abort with a clear
error if non-zero, or avoid activation entirely by replacing the two "python -m
pip install..." calls with "conda run -n backend_env python -m pip install
--upgrade pip" and "conda run -n backend_env python -m pip install -r
requirements.txt"; ensure you perform this change around the existing conda
create/activate and pip install invocations so installs target the intended
"backend_env" environment.
- Around line 192-199: The current block runs "conda create -n sync_env
python=3.12 -y" and "conda activate sync_env" without checking errors; change it
to either use "conda run -n sync_env -- python -m pip install ..." for
idempotent, error-safe execution or add explicit error checks after "conda
create -n sync_env ..." and "conda activate sync_env" (capture and test the exit
code) and log/exit on failure; update the sequence around "conda create -n
sync_env python=3.12 -y", "conda activate sync_env", and the pip install
commands so failures are detected and handled consistently with the backend
setup.
🧹 Nitpick comments (3)
scripts/setup.ps1 (3)
121-136: Add error handling for Miniconda installation.The installation block lacks error handling. If the download or installation fails, subsequent Conda commands will fail with confusing errors. Consider wrapping this in a try-catch and verifying the installation succeeded.
Proposed improvement
if (-not (Test-Path $condaExe)) { Write-Host "Installing Miniconda..." -ForegroundColor Yellow - $installer = "$env:TEMP\miniconda.exe" - Invoke-WebRequest ` - -Uri "https://repo.anaconda.com/miniconda/Miniconda3-latest-Windows-x86_64.exe" ` - -OutFile $installer - - Start-Process -Wait -FilePath $installer -ArgumentList ` - "/InstallationType=JustMe", - "/AddToPath=0", - "/RegisterPython=0", - "/S", - "/D=$condaRoot" - - Remove-Item $installer + try { + $installer = "$env:TEMP\miniconda.exe" + Invoke-WebRequest ` + -Uri "https://repo.anaconda.com/miniconda/Miniconda3-latest-Windows-x86_64.exe" ` + -OutFile $installer -ErrorAction Stop + + Start-Process -Wait -FilePath $installer -ArgumentList ` + "/InstallationType=JustMe", + "/AddToPath=0", + "/RegisterPython=0", + "/S", + "/D=$condaRoot" + + Remove-Item $installer -ErrorAction SilentlyContinue + + if (-not (Test-Path $condaExe)) { + Write-Host "Miniconda installation failed." -ForegroundColor Red + exit 1 + } + Write-Host "Miniconda installed successfully." -ForegroundColor Green + } catch { + Write-Host "Error installing Miniconda: $_" -ForegroundColor Red + exit 1 + } +} else { + Write-Host "Miniconda is already installed." -ForegroundColor Green }
143-144: Add error handling for Conda initialization.If
conda info --basefails orconda-hook.ps1doesn't exist, subsequentconda activatecommands will fail silently or produce confusing errors.Proposed improvement
-$condaBase = & $condaExe info --base -& "$condaBase\shell\condabin\conda-hook.ps1" +$condaBase = & $condaExe info --base 2>&1 +if (-not $condaBase -or -not (Test-Path $condaBase)) { + Write-Host "Failed to determine Conda base path." -ForegroundColor Red + exit 1 +} + +$condaHook = "$condaBase\shell\condabin\conda-hook.ps1" +if (Test-Path $condaHook) { + & $condaHook + Write-Host "Conda initialized successfully." -ForegroundColor Green +} else { + Write-Host "Conda hook script not found at: $condaHook" -ForegroundColor Red + exit 1 +}
84-102: Consider whether system Python 3.12 installation is still needed.With Conda environments now managing their own Python (
python=3.12), the Chocolatey Python 3.12 installation may be redundant. If it's only used for the backend and sync-microservice, you could remove this block to simplify the setup.However, if other parts of the setup or workflow rely on a system Python, keep it.
… handle errors and improve clarity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/setup.ps1`:
- Around line 128-136: Start capturing the Miniconda installer process and check
its exit code after Start-Process by using the -PassThru option (e.g., assign
Start-Process -Wait -PassThru -FilePath $installer ... to a variable), then if
the returned object's ExitCode is non-zero, write an error/verbose message and
stop the script (exit 1) instead of continuing; only call Remove-Item $installer
after confirming a successful (zero) ExitCode so failures are detected early.
🧹 Nitpick comments (2)
scripts/setup.ps1 (2)
143-144: Consider adding validation for Conda initialization.If
conda info --basefails orconda-hook.ps1doesn't exist, the script continues without proper Conda shell integration, which could cause subtle failures later.Proposed validation
$condaBase = & $condaExe info --base +if ($LASTEXITCODE -ne 0 -or -not $condaBase) { + throw "Failed to determine Conda base path" +} + +$condaHook = "$condaBase\shell\condabin\conda-hook.ps1" +if (-not (Test-Path $condaHook)) { + throw "Conda hook script not found at: $condaHook" +} -& "$condaBase\shell\condabin\conda-hook.ps1" +& $condaHook
84-102: Consider whether standalone Python installation is still needed.With the switch to Conda environments (which include their own Python 3.12), the Chocolatey Python 3.12 installation may be redundant. If no other part of the setup requires a system-wide Python, this block could be removed to simplify the script and reduce installation time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/setup.ps1`:
- Around line 128-130: The error check incorrectly references an undefined
variable $installerPath; update the conditional to use the defined $installer
variable instead so the Test-Path and exit-code check operate on the actual
downloaded file (i.e., replace usages of $installerPath with $installer in the
if condition that checks $LASTEXITCODE and Test-Path).
♻️ Duplicate comments (1)
scripts/setup.ps1 (1)
132-141:$LASTEXITCODEis not set byStart-Process; exit code check is ineffective.
Start-Processdoes not populate$LASTEXITCODE. The check on line 139 will always see the exit code from the previous native command (likelyInvoke-WebRequest), not the installer. Use-PassThruto capture the process object and inspect itsExitCodeproperty.Proposed fix
- Start-Process -Wait -FilePath $installer -ArgumentList ` + $process = Start-Process -Wait -PassThru -FilePath $installer -ArgumentList ` "/InstallationType=JustMe", "/AddToPath=0", "/RegisterPython=0", "/S", "/D=$condaRoot" - if ($LASTEXITCODE -ne 0) { - throw "Miniconda installer exited with an error" + if ($process.ExitCode -ne 0) { + throw "Miniconda installer exited with error code $($process.ExitCode)" }
🧹 Nitpick comments (1)
scripts/setup.ps1 (1)
161-162: Consider adding error handling for Conda initialization.If
conda info --basefails or returns an unexpected value, the subsequent hook script invocation will fail with a confusing error. Adding a check would improve debuggability.Suggested improvement
$condaBase = & $condaExe info --base +if ($LASTEXITCODE -ne 0 -or [string]::IsNullOrWhiteSpace($condaBase)) { + throw "Failed to retrieve Conda base path" +} & "$condaBase\shell\condabin\conda-hook.ps1" +if ($LASTEXITCODE -ne 0) { + throw "Failed to initialize Conda shell hook" +}
fixes: #1072
This PR standardizes the Windows environment setup by using Conda instead of python venv for both the backend and sync-microservice.
The goal is to make the setup reliable, reproducible, and consistent across manual and script-based workflows.
Problem
The current manual setup documentation recommends Conda
The PowerShell setup scripts create and activate python venv
This mismatch leads to:
package resolution issues on Windows
broken pip / CLI tools inside venv
confusion for contributors following docs
inconsistent environments between services
In practice, this caused failures even after successful venv activation.
What this PR changes
Replaces python venv usage with Conda environments in PowerShell scripts
Uses the same Conda-based flow for:
backend
sync-microservice
Properly initializes Conda in PowerShell using:
conda info --base
conda-hook.ps1
Ensures the Conda environment is correctly activated before running any commands
Keeps the rest of the setup logic unchanged for minimal disruption
Why Conda
Better dependency resolution on Windows
Avoids broken pip / missing module issues
Matches the manual setup instructions
More predictable for contributors and CI
Impact
No runtime behavior changes
Only affects local development setup on Windows
Improves onboarding and reduces environment-related bugs
Testing
Ran the updated PowerShell scripts on Windows
Verified Conda environment activation
Confirmed backend and sync-microservice start correctly inside the Conda env
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.