Update build pipeline to support the microservice.#492
Update build pipeline to support the microservice.#492rahulharpal1603 merged 2 commits intoAOSSIE-Org:mainfrom
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds CI jobs to build and package a sync microservice per platform, integrates sync artifacts into the publish-and-release and TAURI workflows, restructures PR test steps to build both services, implements frozen vs development startup for the sync service launcher, and enables running the sync server via uvicorn when executed directly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Backend as Backend
participant Utils as microservice.py
participant OS as OS/Process
participant Sync as Sync Microservice
User->>Backend: Start application
Backend->>Utils: start_sync_microservice()
alt Frozen (PyInstaller)
Utils->>OS: Launch PictoPy_Sync[.exe] (subprocess) in PictoPy_Sync dir
note right of OS #D6EAF8: Uses bundled executable\ncaptures stdout/stderr
else Development
Utils->>Utils: Ensure .sync-env venv exists (create if needed)
Utils->>OS: pip install -r requirements.txt (if present)
Utils->>OS: venv-python run uvicorn sync-microservice.main:app --port 8001
note right of OS #E9F7EF: Starts uvicorn via venv Python
end
OS-->>Utils: PID/handle
Utils-->>Backend: Started (True/False)
sequenceDiagram
autonumber
participant GH as GitHub Actions
participant BuildSrv as build-server-*
participant BuildSync as build-sync-microservice-*
participant Publish as publish-tauri
GH->>BuildSrv: Run server build matrix (win/linux/macos)
GH->>BuildSync: Run sync build matrix (win/linux/macos)
BuildSrv-->>GH: Upload `server-artifact` per platform
BuildSync-->>GH: Upload `sync-artifact` per platform
GH->>Publish: Trigger publish-tauri (needs all builds)
Publish->>GH: Download `server-artifact` + `sync-artifact`
Publish->>Publish: Extract artifacts and package Tauri apps
Publish-->>GH: Upload release assets per platform
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
.github/workflows/pr-check-tests.yml (1)
68-72: Update setup-python to v5.actionlint flags v4 as too old on current runners. Use v5 for reliability.
- - name: Set up Python - uses: actions/setup-python@v4 + - name: Set up Python + uses: actions/setup-python@v5 with: python-version: "3.11"backend/app/utils/microservice.py (2)
230-236: Start the dev server via uvicorn and specify the app.
fastapi devrequires the CLI package and a target; current command omits it and may fail. Use uvicorn with explicit app and host/port.- print(python_executable) - cmd = [str(python_executable), "-m", "fastapi", "dev", "--port", "8001"] + host = os.getenv("PICTOPY_SYNC_HOST", "127.0.0.1") + port = os.getenv("PICTOPY_SYNC_PORT", "8001") + cmd = [ + str(python_executable), + "-m", + "uvicorn", + "main:app", + "--host", + host, + "--port", + port, + ]
245-252: Preserve traceback on dev start failure.- except Exception as e: - logger.error(f"Error starting FastAPI service: {e}") + except Exception: + logger.exception("Error starting FastAPI service").github/workflows/build-and-release.yml (2)
307-314: Cross-platform extraction: avoid unzip on Windows.windows-latest often lacks unzip. Use OS-conditional extraction (Expand-Archive) on Windows.
- - name: Extract server files - shell: bash - run: | - cd backend/dist - unzip -o *.zip - rm *.zip - ls -l + - name: Extract server files (Windows) + if: runner.os == 'Windows' + run: | + cd backend/dist + powershell -Command "Expand-Archive -Path (Get-ChildItem -Filter *.zip).FullName -DestinationPath . -Force" + del *.zip + dir + - name: Extract server files (Unix) + if: runner.os != 'Windows' + run: | + cd backend/dist + unzip -o *.zip && rm *.zip && ls -l @@ - - name: Extract sync microservice files - shell: bash - run: | - cd sync-microservice/dist - unzip -o *.zip - rm *.zip - ls -l + - name: Extract sync microservice files (Windows) + if: runner.os == 'Windows' + run: | + cd sync-microservice/dist + powershell -Command "Expand-Archive -Path (Get-ChildItem -Filter *.zip).FullName -DestinationPath . -Force" + del *.zip + dir + - name: Extract sync microservice files (Unix) + if: runner.os != 'Windows' + run: | + cd sync-microservice/dist + unzip -o *.zip && rm *.zip && ls -lAlso applies to: 315-322
29-31: Non-interactive pip uninstall can hang.Add -y to avoid prompts on CI.
- pip uninstall onnxruntime + pip uninstall -y onnxruntime @@ - pip uninstall onnxruntime + pip uninstall -y onnxruntimeAlso applies to: 71-73
🧹 Nitpick comments (3)
backend/app/utils/microservice.py (3)
58-67: Allow overriding bundled executable path.Hardcoding PictoPy_Sync under app_dir makes packaging brittle. Support an override env.
- sync_dir = app_dir / "PictoPy_Sync" + sync_dir = Path(os.getenv("PICTOPY_SYNC_DIR", str(app_dir / "PictoPy_Sync")))
240-242: Log the actual bound URL.- logger.info("Service should be available at http://localhost:8001") + logger.info(f"Service should be available at http://{host}:{port}")
135-137: Optional: wait for readiness before returning True.Consider polling the health endpoint (with timeout) to ensure the service is actually up before signaling success.
📜 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 (4)
.github/workflows/build-and-release.yml(2 hunks).github/workflows/pr-check-tests.yml(1 hunks)backend/app/utils/microservice.py(3 hunks)sync-microservice/main.py(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/build-and-release.yml
144-144: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
184-184: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
224-224: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Tauri Tests
- GitHub Check: Backend Tests
🔇 Additional comments (1)
.github/workflows/pr-check-tests.yml (1)
86-89: Confirm pytest is installed.If not pinned in backend/requirements.txt, this step will fail. Either add it to requirements or install here.
Would you like me to add a minimal backend/requirements-dev.txt and wire it in this job?
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: 3.12 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Bump setup-python to v5 across new sync jobs.
Fixes “runner too old” errors flagged by actionlint.
- - name: Setup Python
- uses: actions/setup-python@v4
+ - name: Setup Python
+ uses: actions/setup-python@v5
with:
python-version: 3.12Also applies to: 184-187, 224-227
🧰 Tools
🪛 actionlint (1.7.7)
144-144: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In .github/workflows/build-and-release.yml around lines 144-147 (and also update
occurrences at 184-187 and 224-227), the workflow uses actions/setup-python@v4
which triggers “runner too old” actionlint errors; change the action reference
to actions/setup-python@v5 in each location (keeping the same python-version
values) so the workflow uses the newer setup-python action.
| - name: Build check(Main backend) | ||
| run: | | ||
| cd backend | ||
| python -m pip install --upgrade pip | ||
| pip install -r requirements.txt | ||
| pyinstaller main.py --name PictoPy_Server --onedir --distpath dist | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure PyInstaller is available for the backend build.
This step assumes pyinstaller is in backend’s requirements. Make it explicit to avoid hidden coupling.
- name: Build check(Main backend)
run: |
cd backend
python -m pip install --upgrade pip
pip install -r requirements.txt
+ pip install pyinstaller
pyinstaller main.py --name PictoPy_Server --onedir --distpath dist📝 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.
| - name: Build check(Main backend) | |
| run: | | |
| cd backend | |
| python -m pip install --upgrade pip | |
| pip install -r requirements.txt | |
| pyinstaller main.py --name PictoPy_Server --onedir --distpath dist | |
| - name: Build check(Main backend) | |
| run: | | |
| cd backend | |
| python -m pip install --upgrade pip | |
| pip install -r requirements.txt | |
| pip install pyinstaller | |
| pyinstaller main.py --name PictoPy_Server --onedir --distpath dist |
🤖 Prompt for AI Agents
.github/workflows/pr-check-tests.yml around lines 73 to 79: the workflow calls
pyinstaller but relies on it being present implicitly in backend requirements;
explicitly install PyInstaller before invoking it to avoid hidden coupling —
update the build step to install PyInstaller (e.g., run python -m pip install
pyinstaller or pip install pyinstaller) immediately after installing
requirements so the pyinstaller command is guaranteed to exist.
| - name: Build check(Sync Microservice) | ||
| run: | | ||
| cd sync-microservice | ||
| pip install -r requirements.txt | ||
| pyinstaller main.py --name PictoPy_Sync_Microservice --onedir --distpath dist | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Install PyInstaller for the sync build too.
- name: Build check(Sync Microservice)
run: |
cd sync-microservice
pip install -r requirements.txt
+ pip install pyinstaller
pyinstaller main.py --name PictoPy_Sync_Microservice --onedir --distpath dist📝 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.
| - name: Build check(Sync Microservice) | |
| run: | | |
| cd sync-microservice | |
| pip install -r requirements.txt | |
| pyinstaller main.py --name PictoPy_Sync_Microservice --onedir --distpath dist | |
| - name: Build check(Sync Microservice) | |
| run: | | |
| cd sync-microservice | |
| pip install -r requirements.txt | |
| pip install pyinstaller | |
| pyinstaller main.py --name PictoPy_Sync_Microservice --onedir --distpath dist |
🤖 Prompt for AI Agents
In .github/workflows/pr-check-tests.yml around lines 80 to 85, the workflow runs
pyinstaller in the sync-microservice step but never installs PyInstaller; update
the step to ensure PyInstaller is available by either (a) running pip install
pyinstaller before invoking pyinstaller, or (b) adding pyinstaller to
sync-microservice/requirements.txt and keeping pip install -r requirements.txt;
ensure the install runs in the same job/step before the pyinstaller command so
the build succeeds.
| try: | ||
| # Check if running as a frozen executable (PyInstaller) | ||
| if getattr(sys, "frozen", False): | ||
| logger.info("Running as frozen executable, using bundled sync microservice...") | ||
| return _start_frozen_sync_service() | ||
|
|
||
| # Development mode - use virtual environment setup | ||
| logger.info("Running in development mode, using virtual environment...") | ||
| return _start_dev_sync_service(sync_service_path) | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Error starting sync microservice: {e}") | ||
| return False |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Log full trace on startup failures.
Use logger.exception to capture stack traces; it eases diagnosing environment/path issues.
- except Exception as e:
- logger.error(f"Error starting sync microservice: {e}")
+ except Exception:
+ logger.exception("Error starting sync microservice")
return False📝 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.
| try: | |
| # Check if running as a frozen executable (PyInstaller) | |
| if getattr(sys, "frozen", False): | |
| logger.info("Running as frozen executable, using bundled sync microservice...") | |
| return _start_frozen_sync_service() | |
| # Development mode - use virtual environment setup | |
| logger.info("Running in development mode, using virtual environment...") | |
| return _start_dev_sync_service(sync_service_path) | |
| except Exception as e: | |
| logger.error(f"Error starting sync microservice: {e}") | |
| return False | |
| try: | |
| # Check if running as a frozen executable (PyInstaller) | |
| if getattr(sys, "frozen", False): | |
| logger.info("Running as frozen executable, using bundled sync microservice...") | |
| return _start_frozen_sync_service() | |
| # Development mode - use virtual environment setup | |
| logger.info("Running in development mode, using virtual environment...") | |
| return _start_dev_sync_service(sync_service_path) | |
| except Exception: | |
| logger.exception("Error starting sync microservice") | |
| return False |
🤖 Prompt for AI Agents
In backend/app/utils/microservice.py around lines 29 to 41, the except block
currently uses logger.error which omits the stack trace; replace
logger.error(f"Error starting sync microservice: {e}") with
logger.exception("Error starting sync microservice") so the full traceback is
logged (no need to interpolate the exception), ensuring failures during startup
record the full stack trace for diagnostics.
| # Start the sync microservice executable | ||
| process = subprocess.Popen( | ||
| [str(sync_executable)], | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| text=True, | ||
| cwd=str(sync_dir), # Set working directory to sync service directory | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid piping stdout/stderr without readers (deadlock risk).
Long-running services can block when pipes fill. Inherit parent stdio or redirect to DEVNULL.
- process = subprocess.Popen(
- [str(sync_executable)],
- stdout=subprocess.PIPE,
- stderr=subprocess.PIPE,
- text=True,
- cwd=str(sync_dir), # Set working directory to sync service directory
- )
+ process = subprocess.Popen(
+ [str(sync_executable)],
+ stdout=None,
+ stderr=None,
+ cwd=str(sync_dir),
+ )📝 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.
| # Start the sync microservice executable | |
| process = subprocess.Popen( | |
| [str(sync_executable)], | |
| stdout=subprocess.PIPE, | |
| stderr=subprocess.PIPE, | |
| text=True, | |
| cwd=str(sync_dir), # Set working directory to sync service directory | |
| ) | |
| # Start the sync microservice executable | |
| process = subprocess.Popen( | |
| [str(sync_executable)], | |
| stdout=None, | |
| stderr=None, | |
| cwd=str(sync_dir), | |
| ) |
🤖 Prompt for AI Agents
In backend/app/utils/microservice.py around lines 74 to 81, the code starts the
sync microservice with stdout and stderr piped which risks deadlock if no
readers consume them; change creation so the child does not pipe output—either
inherit the parent stdio by removing stdout/stderr arguments or explicitly
redirect to subprocess.DEVNULL (or to rotating log files) so the subprocess
cannot block when its output buffers fill.
| @@ -1,4 +1,5 @@ | |||
| from fastapi import FastAPI | |||
| from uvicorn import Config, Server | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make host/port configurable and exit with proper status.
Allow overriding via env vars and propagate uvicorn’s run() result to the process exit code.
+import os
+import sys
from uvicorn import Config, Server
@@
if __name__ == "__main__":
- config = Config(app=app, host="0.0.0.0", port=8001, log_config=None)
+ host = os.getenv("PICTOPY_SYNC_HOST", "0.0.0.0")
+ port = int(os.getenv("PICTOPY_SYNC_PORT", "8001"))
+ config = Config(app=app, host=host, port=port, log_config=None)
server = Server(config)
- server.run()
+ ok = server.run()
+ sys.exit(0 if ok else 1)Also applies to: 19-22
🤖 Prompt for AI Agents
In sync-microservice/main.py around lines 2 and 19-22, make the server host and
port configurable from environment variables (e.g., read HOST with a sensible
default like "0.0.0.0" and PORT defaulting to 8000, converting PORT to int) and
use uvicorn.run(...) with return_exit_status=True so you can capture its
returned exit code; then call sys.exit(exit_code) to propagate the server run
result to the process exit status. Ensure you import os and sys and replace the
hardcoded host/port and current run invocation accordingly.
1 participant
Summary by CodeRabbit
New Features
Tests
Chores