Skip to content

Revert "Migrate evals database when deploying roo-code-website" #6525

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
merged 1 commit into from
Jul 31, 2025
Merged

Revert "Migrate evals database when deploying roo-code-website" #6525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 6 additions & 11 deletions .github/workflows/website-deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ on:
branches:
- main
paths:
- "apps/web-roo-code/**"
- 'apps/web-roo-code/**'
Copy link
Contributor

@roomote roomote bot Jul 31, 2025

Choose a reason for hiding this comment

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

These quote style changes (single to double quotes) seem unrelated to the main purpose of reverting the database migration. Would it be cleaner to keep the formatting as it was, or handle formatting changes in a separate PR?

workflow_dispatch:

env:
Expand All @@ -21,11 +21,11 @@ jobs:
- name: Check if VERCEL_TOKEN exists
id: check
run: |
if [ -n "${{ secrets.VERCEL_TOKEN }}" ]; then
echo "has-vercel-token=true" >> $GITHUB_OUTPUT
else
echo "has-vercel-token=false" >> $GITHUB_OUTPUT
fi
if [ -n "${{ secrets.VERCEL_TOKEN }}" ]; then
echo "has-vercel-token=true" >> $GITHUB_OUTPUT
else
echo "has-vercel-token=false" >> $GITHUB_OUTPUT
fi

deploy:
runs-on: ubuntu-latest
Expand All @@ -36,11 +36,6 @@ jobs:
uses: actions/checkout@v4
- name: Setup Node.js and pnpm
uses: ./.github/actions/setup-node-pnpm
- name: Migrate evals database
run: pnpm db:migrate:production
working-directory: packages/evals
env:
DATABASE_URL: ${{ secrets.EVALS_DATABASE_URL }}
- name: Install Vercel CLI
run: npm install --global vercel@canary
- name: Pull Vercel Environment Information
Expand Down
5 changes: 3 additions & 2 deletions packages/evals/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
"drizzle-kit:production": "dotenvx run -f .env.production -- tsx node_modules/drizzle-kit/bin.cjs",
"db:generate": "pnpm drizzle-kit generate",
"db:migrate": "pnpm drizzle-kit migrate",
"db:migrate:production": "pnpm drizzle-kit:production migrate",
"db:push:test": "pnpm drizzle-kit:test push",
"db:push": "pnpm drizzle-kit push",
Copy link
Contributor

@roomote roomote bot Jul 31, 2025

Choose a reason for hiding this comment

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

I notice this changes the migration strategy from db:migrate commands to db:push commands. Could you clarify why this change in approach?

The push command typically overwrites the database schema rather than applying incremental migrations, which could be risky in production environments. Is this change related to the issue that caused the original PR to fail?

"db:test:push": "pnpm drizzle-kit:test push",
"db:production:push": "pnpm drizzle-kit:production push",
"db:up": "dotenvx run -f .env.development .env.local -- docker compose up -d db",
"db:down": "dotenvx run -f .env.development .env.local -- docker compose down db",
"redis:up": "dotenvx run -f .env.development .env.local -- docker compose up -d redis",
Expand Down
1 change: 1 addition & 0 deletions packages/evals/src/db/migrations/0001_add_timeout_to_runs.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE "runs" ADD COLUMN "timeout" integer DEFAULT 5 NOT NULL;
Copy link
Contributor

@roomote roomote bot Jul 31, 2025

Choose a reason for hiding this comment

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

This migration file appears to be a duplicate of the existing 0001_lowly_captain_flint.sql file. Both files contain the exact same SQL:

ALTER TABLE "runs" ADD COLUMN "timeout" integer DEFAULT 5 NOT NULL;

Having duplicate migration files with the same content will cause conflicts. Is this intentional? If the original PR deleted this file and you're reverting that deletion, we might need to handle the existing 0001_lowly_captain_flint.sql file differently.

Loading