fix (db): update db-migration to create swaps i823#1301
Conversation
JustKuzya
commented
Jun 4, 2026
JustKuzya
commented
- add guide for testing the database upgrade with containerized Postgres DBMS
- create migration script for swaps
- verify that the script works for SQLite and containerized Postgres
keithmanville
left a comment
keithmanville
left a comment
There was a problem hiding this comment.
I was expecting to see the JobSwaps ORM object in db/models/jobs.py and changes to the response schema in v1/jobs/schema.py. Did you forget to commit? It looks like you had this done if you generated the alembic script from the ORM.
- add guide for testing the database upgrade with containerized postgres DBMS - create migration script for swaps - verify that the script works for SQLite and containerized Posgres
Lint and format the frontend using npm run lint and npm run format from the src/frontend directory. Perform check only using npm run lint:check and npm run format:check. Updated `DEVELOPER.md` to note lint and format rules. Updated all frontend packages. Note: eslint is incompatible with node v23. Added .nvmrc so nvm can be used to to switch to node v22. Added short explanation in frontend README.
Added frontend-checks.yml which runs the lint:check and format:check for PRs.
- added method doc-strings - changed the commit messafge structure to space-less one
- create ORM mapping in model - regenerate upgrade script
- update the model with job_id+
| ) # FK to plugin_files | ||
|
|
||
| # PK constraints settings | ||
| _table_args__ = ( |
There was a problem hiding this comment.
typo: should be __table_args__
| __tablename__ = "job_swaps" | ||
|
|
||
| # Table fields | ||
| job_id: Mapped["Job"] = mapped_column( |
There was a problem hiding this comment.
Let's use job_resource_id instead of job_id to match the other ORM models like EntrypointJob and ExperimentJob.
The type should be an integer since it is an ID not the job ORM class.
| # PK constraints settings | ||
| _table_args__ = ( | ||
| PrimaryKeyConstraint("job_id", "swap_name"), | ||
| ForeignKeyConstraint( |
There was a problem hiding this comment.
job_id should be with jobs.resource_id and plugin_file_resource_snapshot_id with plugin_files.resource_snapshot_id.
But in the end I don't think we need the FK Constraints here, as long as they are defined in the mapped_columns
| ForeignKey("plugin_files.resource_snapshot_id"), | ||
| init=False, | ||
| ) # FK to plugin_files | ||
|
|
There was a problem hiding this comment.
need to add relationship with Jobs. Also requires the entry in Jobs ORM object:
job_swaps: Mapped[list["JobSwap"]] = relationship(
init=False,
back_populates="job",
)
| from alembic import op | ||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision = '30e74c5564b3' |
There was a problem hiding this comment.
seeing single quotes. need to run ruff on this file.
There was a problem hiding this comment.
I see in the latest version all of the changes you have suggested seem to be applied.
keithmanville
force-pushed
the
jk/db-swaps-upgrade-i823
branch
from
9ed9c34 to
92a7e10
Compare
|
Left a review, but since changes were all minor, I went ahead and made and committed them. Also change PR target to |
3 participants
