Conversation
Backfill jobs.workflow, users.settings.quick_analyze_workflow, and mongo analyses.workflow, then remove the Workflow._missing_ alias, the claim-time pathoscope_bowtie fallback, and PATHOSCOPE_TASK_NAMES shims.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request renames the legacy pathoscope_bowtie workflow to pathoscope across the codebase. It includes a migration script to update existing records in PostgreSQL and MongoDB, alongside updates to models, utilities, and test snapshots. Feedback was provided regarding the migration tests, specifically recommending the use of bind parameters instead of f-string interpolation for SQL queries to improve security and robustness.
| settings = ( | ||
| '{"quick_analyze_workflow": "' + quick_analyze_workflow + '"}' | ||
| if quick_analyze_workflow is not None | ||
| else "{}" | ||
| ) | ||
| async with AsyncSession(ctx.pg) as session: | ||
| result = await session.execute( | ||
| text( | ||
| f""" | ||
| INSERT INTO users ( | ||
| handle, active, email, force_reset, | ||
| invalidate_sessions, last_password_change, password, settings | ||
| ) | ||
| VALUES ( | ||
| :handle, true, '', false, false, :now, :pw, '{settings}'::jsonb | ||
| ) | ||
| RETURNING id | ||
| """, | ||
| ), | ||
| {"handle": handle, "now": arrow.utcnow().naive, "pw": b"hashed"}, | ||
| ) |
There was a problem hiding this comment.
The manual construction of the JSON string and its interpolation into the SQL query using an f-string is fragile and prone to errors (e.g., if the workflow name contains quotes). It is better to use bind parameters for the settings field and let the database driver handle the serialization. This also avoids potential SQL injection issues, even if this is only a test file.
Replace the f-string-interpolated JSON with a CAST(:settings AS jsonb) bind so the helper handles arbitrary workflow names without quoting hazards.
igboyes
deleted the
igboyes/vir-2310-rename-pathoscope_bowtie-to-pathoscope-everywhere
branch
1 participant
Summary
pathoscope_bowtieshims and aliases now that the backfill migration has been written, leavingpathoscopeas the single canonical workflow name across PostgreSQL jobs, user settings (JSONB), MongoDB analyses, enums, and API modelsrev_3g8rzbqj6k69) that renames any remainingpathoscope_bowtievalues in the three data stores so the shims can be safely droppedWorkflow._missing_compatibility hook, thePATHOSCOPE_TASK_NAMESlist, theget_workflow_namehelper, the job-claim alias query, and the corresponding test coverage for those shims