-
-
Notifications
You must be signed in to change notification settings - Fork 806
Add conda one-click installer script and documentation #389
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?
Conversation
|
@Akilesh-Sahaj is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a new Conda one‑click installer script and updates README: documents prerequisites, usage, environment variables, and post‑install steps. The script creates or reuses a Conda env, upgrades packaging tools, installs backend deps (editable), optional dev tools, and conditionally installs frontend/extension via npm. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Script as oneclick-conda-install.sh
participant Conda
participant Env as Conda Env
participant Python as Backend (pip)
participant NPM as Frontend (npm / Node.js)
User->>Script: ./oneclick-conda-install.sh
Script->>Conda: require conda available
Script->>Conda: conda env list / check Env exists
alt Env missing
Script->>Conda: conda create (name, Python version)
else Env exists
Script->>Env: reuse existing env
end
Script->>Env: conda run python -m pip install -U pip setuptools wheel
Script->>Python: install backend (editable) and dev tools (e.g., pre-commit)
Script->>NPM: check npm and Node.js version (v18+)
alt npm & Node.js OK
Script->>NPM: npm install frontend & extension deps
else npm/Node.js missing or too old
Note right of Script #f9f0c1: Skip frontend install (warn)
end
Script-->>User: print activation and run instructions (uvicorn / npm)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
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.
Review by RecurseML
🔍 Review performed on 402039f..9df3b8d
| Severity | Location | Issue | Delete |
|---|---|---|---|
| surfsense_web/oneclick-conda-install.sh:77 | Undefined variable causes crash |
✅ Files analyzed, no issues (1)
• README.md
9df3b8d to
c0e8f4d
Compare
c0e8f4d to
2c30139
Compare
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
♻️ Duplicate comments (2)
surfsense_web/oneclick-conda-install.sh (2)
26-29: PointPROJECT_ROOTat the repo root before deriving subpaths.
PROJECT_ROOTcurrently resolves to.../surfsense_web, soFRONTEND_DIRbecomes.../surfsense_web/surfsense_webandEXTENSION_DIRbecomes.../surfsense_web/surfsense_browser_extension. Both directories are invalid, so everycd "${FRONTEND_DIR}"/"${EXTENSION_DIR}"will fail. SetPROJECT_ROOTto the repository root (e.g.,cd "$(dirname "${BASH_SOURCE[0]}")/..") before defining the other paths.
76-100: DefineBACKEND_DIR(and reuse it consistently).
BACKEND_DIRis never set, sopip install -e "${BACKEND_DIR}"expands to an empty string and exits immediately; the post-install instructions using${BACKEND_DIR}are broken for the same reason. Declare it alongside the other path constants (e.g.,readonly BACKEND_DIR="${PROJECT_ROOT}/surfsense_backend").
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)surfsense_web/oneclick-conda-install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
surfsense_web/oneclick-conda-install.sh
[warning] 26-26: Declare and assign separately to avoid masking return values.
(SC2155)
|
@MODSetter requesting review |
|
Planning on reviewing this later - but over a quick glance I think the doc need some further changes. Looking at the script I assume the script only depends on the conda installed? If that is the case it would be better to make the conda installation write out of quote block of docker & manual installation to help clarify things. |
|
Okay the script seems not working on my machine - as RecurseML have pointed out I think there is this unbound variable issue on line 77. It was marked as resolved though... |
@Aki-07 Can you please fix this. |
|
@MODSetter Sure, will get it fixed in some time |
Reordered the installation options in README.md to highlight the Conda one-click installer as the recommended method for most users. This change aims to clarify and streamline the setup process.
|
@MODSetter requesting for review |
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
🧹 Nitpick comments (6)
README.md (3)
151-151: Standardize spelling (customize vs customise).Use consistent regional spelling across the README. Suggest American English to match the rest.
- - Use `SURFSENSE_ENV_NAME` and `SURFSENSE_PYTHON_VERSION` env vars to customise the Conda environment. + - Use `SURFSENSE_ENV_NAME` and `SURFSENSE_PYTHON_VERSION` env vars to customize the Conda environment.
154-156: Hyphenate compound adjective.“Full-stack” should be hyphenated when used as a compound adjective.
- - Flexible deployment options (full stack or core services only) + - Flexible deployment options (full-stack or core services only)
144-145: Minor wording polish.Avoid the slash construction; it reads cleaner as “backend and frontend.”
-1. **Conda One-Click Installer (Recommended for most users)** - Create a ready-to-use Conda environment and install backend/frontend dependencies automatically. +1. **Conda One-Click Installer (Recommended for most users)** - Create a ready-to-use Conda environment and install backend and frontend dependencies automatically.oneclick-conda-install.sh (3)
26-30: Address SC2155 and make script dir explicit.Declare and assign separately to avoid masking return values; keep PROJECT_ROOT semantics unchanged.
-readonly PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +readonly SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +readonly PROJECT_ROOT="$SCRIPT_DIR" readonly FRONTEND_DIR="${PROJECT_ROOT}/surfsense_web" readonly EXTENSION_DIR="${PROJECT_ROOT}/surfsense_browser_extension" readonly BACKEND_DIR="${PROJECT_ROOT}/surfsense_backend"
83-91: Check Node.js version, prefer npm ci, and guard missing dirs.
- Ensure Node >= 18 as documented.
- Use npm ci when lockfiles exist for reproducibility.
- Skip missing frontend/extension dirs instead of hard failing.
-if command -v npm >/dev/null 2>&1; then - info "Installing frontend dependencies (surfsense_web)…" - (cd "${FRONTEND_DIR}" && npm install) - - info "Installing browser extension dependencies (surfsense_browser_extension)…" - (cd "${EXTENSION_DIR}" && npm install) +if command -v npm >/dev/null 2>&1 && command -v node >/dev/null 2>&1; then + NODE_MAJOR="$(node -v | sed -E 's/^v([0-9]+).*/\1/')" + if [[ "${NODE_MAJOR}" -lt 18 ]]; then + warn "Node.js v${NODE_MAJOR} detected; Node 18+ is recommended. Skipping frontend install." + else + if [[ -d "${FRONTEND_DIR}" ]]; then + info "Installing frontend dependencies (surfsense_web)…" + if [[ -f "${FRONTEND_DIR}/package-lock.json" ]]; then + (cd "${FRONTEND_DIR}" && npm ci) + else + (cd "${FRONTEND_DIR}" && npm install) + fi + else + warn "Frontend directory not found at '${FRONTEND_DIR}'; skipping web UI install." + fi + + if [[ -d "${EXTENSION_DIR}" ]]; then + info "Installing browser extension dependencies (surfsense_browser_extension)…" + if [[ -f "${EXTENSION_DIR}/package-lock.json" ]]; then + (cd "${EXTENSION_DIR}" && npm ci) + else + (cd "${EXTENSION_DIR}" && npm install) + fi + else + warn "Extension directory not found at '${EXTENSION_DIR}'; skipping extension install." + fi + fi else warn "npm not found; skipping frontend dependency installation. Install Node.js 18+ to enable the web UI." fi
80-82: Optional: install Git hooks automatically.If you want devs to get pre-commit hooks by default, add “pre-commit install” when git is available.
info "Installing optional developer helpers…" conda_run python -m pip install "pre-commit>=3.8.0" +if command -v git >/dev/null 2>&1; then + (cd "${PROJECT_ROOT}" && conda_run pre-commit install || true) +fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)oneclick-conda-install.sh(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~144-~144: There might be a mistake here.
Context: ...end/frontend dependencies automatically. - Requires Miniconda/Anaconda plus Node.js...
(QB_NEW_EN)
[uncategorized] ~156-~156: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... file - Flexible deployment options (full stack or core services only) - No need to ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~161-~161: Do not mix variants of the same word (‘customize’ and ‘customise’) within a single text.
Context: ...ore control over their setup or need to customize their deployment. Both installation gu...
(EN_WORD_COHERENCY)
🪛 Shellcheck (0.11.0)
oneclick-conda-install.sh
[warning] 26-26: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (3)
oneclick-conda-install.sh (2)
96-101: Verify backend entrypoint and working dir in post-install instructions.Confirm uvicorn target module/path is correct and matches the repo layout. If incorrect, newcomers will get import errors.
- Does app.app:app exist under ${BACKEND_DIR}?
- If not, update the command accordingly (e.g., python -m uvicorn surfsense.app:app).
Also applies to: 104-104
78-78: Earlier BACKEND_DIR issue appears resolved.BACKEND_DIR is now defined (Line 29) and used here; this should eliminate the prior “unbound variable” failure.
Please re-run the script in the environment that previously failed and confirm it now passes the BACKEND install step.
README.md (1)
146-150: Script path verified
Theoneclick-conda-install.shscript exists at the repository root; no updates required.
05388d7 to
47813c3
Compare
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
🧹 Nitpick comments (3)
oneclick-conda-install.sh (3)
26-30: Make directory paths configurable; avoid assuming script’s folder equals repo root.If this script lives under surfsense_web (as in PR summary), FRONTEND_DIR becomes “surfsense_web/surfsense_web”. Allow env overrides to decouple layout.
-readonly PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -readonly FRONTEND_DIR="${PROJECT_ROOT}/surfsense_web" -readonly EXTENSION_DIR="${PROJECT_ROOT}/surfsense_browser_extension" -readonly BACKEND_DIR="${PROJECT_ROOT}/surfsense_backend" +# Resolve script directory; allow directory overrides via env vars. +_SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +readonly PROJECT_ROOT="${SURFSENSE_PROJECT_ROOT:-${_SCRIPT_DIR}}" +readonly FRONTEND_DIR="${SURFSENSE_FRONTEND_DIR:-${PROJECT_ROOT}/surfsense_web}" +readonly EXTENSION_DIR="${SURFSENSE_EXTENSION_DIR:-${PROJECT_ROOT}/surfsense_browser_extension}" +readonly BACKEND_DIR="${SURFSENSE_BACKEND_DIR:-${PROJECT_ROOT}/surfsense_backend}"Optionally, detect repo root via git and fall back to script dir:
REPO_ROOT="$(git -C "$_SCRIPT_DIR" rev-parse --show-toplevel 2>/dev/null || echo "$_SCRIPT_DIR")"
26-26: Shellcheck SC2155: split readonly assignment.Minor: assign first, then mark readonly to avoid masking return codes.
-readonly PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +_SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +readonly PROJECT_ROOT="${SURFSENSE_PROJECT_ROOT:-${_SCRIPT_DIR}}"(Based on static analysis)
96-111: Improve post‑install UX: include no‑activate run example.Add an example that runs uvicorn via conda run for users who don’t want to activate the env.
2. Start the backend API: (cd "${BACKEND_DIR}" && uvicorn app.app:app --reload) + # or without activating: + conda run -n ${ENV_NAME} uvicorn app.app:app --reload
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
oneclick-conda-install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
oneclick-conda-install.sh
[warning] 26-26: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (1)
oneclick-conda-install.sh (1)
77-82: No changes required. PROJECT_ROOT and BACKEND_DIR are correctly defined at the top and guarded before use; the unbound‐variable error in your test arose because you ran the checks without sourcing oneclick-conda-install.sh.
47813c3 to
184df08
Compare
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: 0
🧹 Nitpick comments (4)
oneclick-conda-install.sh (4)
26-29: Address ShellCheck SC2155: split readonly assignment.Avoid assign-at-declare to silence SC2155 and improve readability.
-readonly PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +readonly PROJECT_ROOT readonly FRONTEND_DIR="${PROJECT_ROOT}/surfsense_web" readonly EXTENSION_DIR="${PROJECT_ROOT}/surfsense_browser_extension" readonly BACKEND_DIR="${PROJECT_ROOT}/surfsense_backend"
54-56: Make env detection more robust across conda output formats.Text parsing can be brittle. Consider using conda info to list env names only.
-conda env list | awk '{print $1}' | grep -Fxq "$ENV_NAME" +conda info --envs | awk 'NR>2 && NF {print $1}' | grep -Fxq "$ENV_NAME"Optionally, use JSON with jq if available for maximum robustness.
86-115: Prefer npm ci when lockfile exists; keeps installs reproducible.Use npm ci if package-lock.json is present; fall back to npm install otherwise.
- (cd "${FRONTEND_DIR}" && npm install) + (cd "${FRONTEND_DIR}" && if [[ -f package-lock.json ]]; then npm ci; else npm install; fi) ... - (cd "${EXTENSION_DIR}" && npm install) + (cd "${EXTENSION_DIR}" && if [[ -f package-lock.json ]]; then npm ci; else npm install; fi)
2-2: Add a simple error trap for easier debugging when a step fails.Helpful with set -euo to surface failing line/command.
-set -euo pipefail +set -euo pipefail +trap 'echo -e "\033[1;31m[ERROR]\033[0m Failed at line $LINENO: $BASH_COMMAND" >&2' ERR
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
oneclick-conda-install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
oneclick-conda-install.sh
[warning] 26-26: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (2)
oneclick-conda-install.sh (2)
62-115: Overall: solid installer; previous unbound var fixed and guards look good.Conda env creation/reuse, pip upgrades, backend install with dir check, and guarded frontend installs (Node ≥18) are correct. Nice use of set -euo pipefail and conda run.
123-125: No change required for uvicorn invocation:BACKEND_DIRis set tosurfsense_backend, andsurfsense_backend/app/app.pydefinesapp = FastAPI(...), souvicorn app.app:app --reloadcorrectly loads the ASGI app.
|
@NatsumeRyuhane Can you give this a go now. |
Description
Motivation and Context
FIX Feature Request: one-click installer with conda #299
FIX #
Changes Overview
Screenshots
API Changes
Types of changes
Testing
Checklist:
High-level PR Summary
This PR introduces an experimental Conda-based one-click installer script (
oneclick-conda-install.sh) to simplify local setup for contributors. The script automates the creation of a Conda environment, installs backend dependencies in editable mode, and handles frontend dependencies for both the web app and browser extension when Node.js is available. The README is updated with installation instructions including environment variable customization options for the Conda environment name and Python version.⏱️ Estimated Review Time: 5-15 minutes
💡 Review Order Suggestion
surfsense_web/oneclick-conda-install.shREADME.mdSummary by CodeRabbit
New Features
Documentation