Skip to content

Staging #104

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.

Sign up for GitHub

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

Merged
gkorland merged 20 commits into from
Aug 21, 2025
Merged

Staging #104

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
7cf175f
fix docker with fastapi
gkorland Aug 21, 2025
26b616a
Merge pull request #102 from FalkorDB/fix-docker
gkorland Aug 21, 2025
9fd9875
clean flask
gkorland Aug 21, 2025
5e9ba19
Merge branch 'main' into staging
gkorland Aug 21, 2025
101b012
Merge branch 'staging' into remove-flask
gkorland Aug 21, 2025
701db7f
Merge pull request #103 from FalkorDB/remove-flask
gkorland Aug 21, 2025
56b9c36
update deps
gkorland Aug 21, 2025
708de13
Merge branch 'staging' into deps
gkorland Aug 21, 2025
1b01b6a
Merge pull request #105 from FalkorDB/deps
gkorland Aug 21, 2025
e655e75
clean lint
gkorland Aug 21, 2025
9c2029c
fix lints
gkorland Aug 21, 2025
9d42263
clean lints
gkorland Aug 21, 2025
c279334
fix
gkorland Aug 21, 2025
890876a
fix tests
gkorland Aug 21, 2025
61897fe
fix tests
gkorland Aug 21, 2025
80d9315
fix tests
gkorland Aug 21, 2025
349f937
Merge pull request #106 from FalkorDB/lint-fixes
gkorland Aug 21, 2025
c0639c7
add design
gkorland Aug 21, 2025
5ce6dda
add header
gkorland Aug 21, 2025
d7141a1
Merge pull request #107 from FalkorDB/design
gkorland Aug 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
# Use a single stage build with FalkorDB base image
# Multi-stage build: Start with Python 3.12 base
FROM python:3.12-bookworm as python-base

# Main stage: Use FalkorDB base and copy Python 3.12
FROM falkordb/falkordb:latest

ENV PYTHONUNBUFFERED=1 \
Expand All @@ -7,12 +10,15 @@ ENV PYTHONUNBUFFERED=1 \

USER root

# Install Python and pip, netcat for wait loop in start.sh
# Copy Python 3.12 from the python base image
COPY --from=python-base /usr/local /usr/local

# Install netcat for wait loop in start.sh
RUN apt-get update && apt-get install -y \
python3 \
python3-pip \
netcat-openbsd \
&& rm -rf /var/lib/apt/lists/*
&& rm -rf /var/lib/apt/lists/* \
&& ln -sf /usr/local/bin/python3.12 /usr/bin/python3 \
&& ln -sf /usr/local/bin/python3.12 /usr/bin/python

Comment on lines 17 to 22

@coderabbitai coderabbitai Bot Aug 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Install required tooling in one layer; add missing essentials and tighten apt flags

We use curl later but it’s not installed. Also add ca-certificates and gnupg for NodeSource, and avoid recommends to keep the image smaller. Create python symlinks after installing.

Apply:

-RUN apt-get update && apt-get install -y \
-    netcat-openbsd \
-    && rm -rf /var/lib/apt/lists/* \
+RUN apt-get update && apt-get install -y --no-install-recommends \
+    netcat-openbsd curl ca-certificates gnupg \
+  && rm -rf /var/lib/apt/lists/* \
     && ln -sf /usr/local/bin/python3.12 /usr/bin/python3 \
     && ln -sf /usr/local/bin/python3.12 /usr/bin/python
📝 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.

Suggested change
RUN apt-get update && apt-get install -y \
python3 \
python3-pip \
netcat-openbsd \
&& rm -rf /var/lib/apt/lists/*
&& rm -rf /var/lib/apt/lists/* \
&& ln -sf /usr/local/bin/python3.12 /usr/bin/python3 \
&& ln -sf /usr/local/bin/python3.12 /usr/bin/python
RUN apt-get update && apt-get install -y --no-install-recommends \
netcat-openbsd curl ca-certificates gnupg \
&& rm -rf /var/lib/apt/lists/* \
&& ln -sf /usr/local/bin/python3.12 /usr/bin/python3 \
&& ln -sf /usr/local/bin/python3.12 /usr/bin/python
🤖 Prompt for AI Agents 
In Dockerfile around lines 17 to 22, the RUN instruction installs only
netcat-openbsd and then creates python symlinks, but it is missing curl,
ca-certificates and gnupg (needed for NodeSource), and it should avoid
installing recommends and create symlinks after packages are installed; update
the single RUN layer to apt-get update && apt-get install -y
--no-install-recommends curl ca-certificates gnupg netcat-openbsd (and any other
required essentials), remove apt lists after install, and then create the python
symlinks (ln -sf ...) in the same layer so the image stays small and has the
necessary tooling.

WORKDIR /app

Expand All @@ -25,6 +31,25 @@ COPY Pipfile Pipfile.lock ./
# Install Python dependencies from Pipfile
RUN PIP_BREAK_SYSTEM_PACKAGES=1 pipenv sync --system

# Install Node.js (Node 22) so we can build the frontend inside the image.
# Use NodeSource setup script to get a recent Node version on Debian-based images.
RUN curl -fsSL https://deb.nodesource.com/setup_22.x | bash - \
&& apt-get update && apt-get install -y nodejs \
&& rm -rf /var/lib/apt/lists/*

Comment on lines +34 to +39

@coderabbitai coderabbitai Bot Aug 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

NodeSource setup lacks curl/gnupg/CA packages and repeats apt metadata

The step will fail on images without curl/gnupg. Also collapse updates into one and use --no-install-recommends.

Apply:

-RUN curl -fsSL https://deb.nodesource.com/setup_22.x | bash - \
-    && apt-get update && apt-get install -y nodejs \
-    && rm -rf /var/lib/apt/lists/*
+RUN curl -fsSL https://deb.nodesource.com/setup_22.x | bash - \
+  && apt-get update \
+  && apt-get install -y --no-install-recommends nodejs \
+  && rm -rf /var/lib/apt/lists/*

Note: This assumes curl/gnupg/ca-certificates were installed earlier (see previous comment). If you prefer fully self-contained step, include them here instead.

📝 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.

Suggested change
# Install Node.js (Node 22) so we can build the frontend inside the image.
# Use NodeSource setup script to get a recent Node version on Debian-based images.
RUN curl -fsSL https://deb.nodesource.com/setup_22.x | bash - \
&& apt-get update && apt-get install -y nodejs \
&& rm -rf /var/lib/apt/lists/*
# Install Node.js (Node 22) so we can build the frontend inside the image.
# Use NodeSource setup script to get a recent Node version on Debian-based images.
RUN curl -fsSL https://deb.nodesource.com/setup_22.x | bash - \
&& apt-get update \
&& apt-get install -y --no-install-recommends nodejs \
&& rm -rf /var/lib/apt/lists/*
🤖 Prompt for AI Agents 
In Dockerfile around lines 34 to 39, the NodeSource setup step assumes
curl/gnupg/ca-certificates exist and runs apt-get update twice; modify the step
to ensure required packages (curl, gnupg, ca-certificates) are installed if not
already, collapse apt-get update/install into a single apt-get update && apt-get
install -y --no-install-recommends <packages> sequence, then run the NodeSource
setup script and install nodejs, and finally rm -rf /var/lib/apt/lists/* to
clean up; alternatively if those packages are guaranteed earlier, remove the
redundant apt-get update and add --no-install-recommends to the nodejs install
line and keep apt lists cleanup.

# Copy only frontend package files so Docker can cache npm installs when
# package.json / package-lock.json don't change.
COPY app/package*.json ./app/

# Install frontend dependencies (reproducible install using package-lock)
RUN if [ -f ./app/package-lock.json ]; then \
npm --prefix ./app ci --no-audit --no-fund; \
elif [ -f ./app/package.json ]; then \
npm --prefix ./app install --no-audit --no-fund; \
else \
echo "No frontend package.json found, skipping npm install"; \
fi

# Copy application code
COPY . .

Expand All @@ -34,6 +59,5 @@ RUN chmod +x /start.sh

EXPOSE 5000 6379 3000


# Use start.sh as entrypoint
ENTRYPOINT ["/start.sh"]
13 changes: 11 additions & 2 deletions start.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,14 @@ while ! nc -z "$FALKORDB_HOST" "$FALKORDB_PORT"; do
done


echo "FalkorDB is up - launching Flask..."
exec python3 -m flask --app api.index run --host=0.0.0.0 --port=5000
echo "FalkorDB is up - launching FastAPI..."
# Determine whether to run in reload (debug) mode. The project uses FLASK_DEBUG
# environment variable historically; keep compatibility by honoring it here.
if [ "${FLASK_DEBUG:-False}" = "True" ] || [ "${FLASK_DEBUG:-true}" = "true" ]; then
RELOAD_FLAG="--reload"
else
RELOAD_FLAG=""
fi

echo "FalkorDB is up - launching FastAPI (uvicorn)..."
exec uvicorn api.index:app --host 0.0.0.0 --port 5000 $RELOAD_FLAG

@coderabbitai coderabbitai Bot Aug 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Ensure uvicorn reloader dependencies and module path exist

  • uvicorn --reload requires watchfiles (or standard extras). Verify Pipfile includes uvicorn[standard] or watchfiles to avoid runtime errors.
  • Verify api/index.py exports app.

Run from repo root:

🏁 Script executed:

#!/bin/bash
# Check FastAPI app and uvicorn deps
rg -nP '^\s*app\s*=\s*FastAPI\(' -C2 --glob 'api/**.py'
rg -nP '^\s*from\s+fastapi\s+import\s+FastAPI' --glob 'api/index.py'
rg -nP '^\s*uvicorn(\[standard\])?' Pipfile Pipfile.lock || rg -n 'watchfiles' Pipfile Pipfile.lock

Length of output: 538

🏁 Script executed:

#!/bin/bash
# Verify that api/index.py exports an app instance
rg -nP '^\s*app\s*=' --glob api/index.py
rg -n 'create_app' --glob api/index.py

Length of output: 203

Add Uvicorn reload dependencies to Pipfile

  • Verified that api/index.py correctly exports app via create_app(); no changes needed there.
  • However, the Pipfile currently has: 
    [packages]
uvicorn = "~=0.32.0"
    This does not include the extras required for --reload. Without uvicorn[standard] or the watchfiles package, using --reload will fail at runtime.

Action Items:

  • In Pipfile, update the uvicorn entry to include the standard extras (or separately add watchfiles): 
    [packages]
- uvicorn = "~=0.32.0"
+ uvicorn = {version = "~=0.32.0", extras = ["standard"]}
  • Run pipenv lock to update Pipfile.lock.
🤖 Prompt for AI Agents 
In start.sh around line 30 the script runs uvicorn with potential live reload
via $RELOAD_FLAG but the project's Pipfile only lists uvicorn without extras;
update the Pipfile to specify uvicorn with the standard extras
(uvicorn[standard] = "~=0.32.0") or add watchfiles to [packages] so --reload
works, then run pipenv lock to regenerate Pipfile.lock.