From 16bc2f4f60351465f64481bbe784bee75ba14f31 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Tue, 14 Jan 2025 10:56:20 -0500 Subject: [PATCH 1/5] Fix name of times-square CLI in cli environment --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 420692d..faea677 100644 --- a/tox.ini +++ b/tox.ini @@ -104,7 +104,7 @@ commands = [testenv:cli] description = Run command-line tool against a test database commands = - timessquare {posargs} + times-square {posargs} setenv = TS_ENVIRONMENT_URL = https://test.example.com TS_PATH_PREFIX = /times-square/api From dfd3b5b88e6cef02525be3f3000891163c4fa13c Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Tue, 14 Jan 2025 11:44:21 -0500 Subject: [PATCH 2/5] Adopt UnicodeText column types where useful In Postgres there is no functional or performance difference between VARCHAR and TEXT columns. Therefore we're removing arbitrary constraints on many string columns by migrating to TEXT column types. This both lets us test database migrations and also prevents future issues where a column needs to have strings longer than the arbitrarily-set column type provides. --- ...3b_adopt_text_column_types_where_useful.py | 139 ++++++++++++++++++ src/timessquare/dbschema/page.py | 16 +- 2 files changed, 147 insertions(+), 8 deletions(-) create mode 100644 alembic/versions/20250114_1620_2a3bb5a5933b_adopt_text_column_types_where_useful.py diff --git a/alembic/versions/20250114_1620_2a3bb5a5933b_adopt_text_column_types_where_useful.py b/alembic/versions/20250114_1620_2a3bb5a5933b_adopt_text_column_types_where_useful.py new file mode 100644 index 0000000..ac17b67 --- /dev/null +++ b/alembic/versions/20250114_1620_2a3bb5a5933b_adopt_text_column_types_where_useful.py @@ -0,0 +1,139 @@ +"""Adopt TEXT column types where useful. + +In Postgres there is not functional or performance difference between VARCHAR +and TEXT columns. Therefore we're removing arbitrary constraints on many +string columns by migrating to TEXT column types. + +Revision ID: 2a3bb5a5933b +Revises: 487195933fee +Create Date: 2025-01-14 16:20:45.553562+00:00 +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "2a3bb5a5933b" +down_revision: str | None = "487195933fee" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + op.alter_column( + "pages", + "uploader_username", + existing_type=sa.VARCHAR(length=64), + type_=sa.UnicodeText(), + existing_nullable=True, + ) + op.alter_column( + "pages", + "github_owner", + existing_type=sa.VARCHAR(length=255), + type_=sa.UnicodeText(), + existing_nullable=True, + ) + op.alter_column( + "pages", + "github_repo", + existing_type=sa.VARCHAR(length=255), + type_=sa.UnicodeText(), + existing_nullable=True, + ) + op.alter_column( + "pages", + "repository_path_prefix", + existing_type=sa.VARCHAR(length=2048), + type_=sa.UnicodeText(), + existing_nullable=True, + ) + op.alter_column( + "pages", + "repository_display_path_prefix", + existing_type=sa.VARCHAR(length=2048), + type_=sa.UnicodeText(), + existing_nullable=True, + ) + op.alter_column( + "pages", + "repository_path_stem", + existing_type=sa.VARCHAR(length=255), + type_=sa.UnicodeText(), + existing_nullable=True, + ) + op.alter_column( + "pages", + "repository_source_extension", + existing_type=sa.VARCHAR(length=255), + type_=sa.UnicodeText(), + existing_nullable=True, + ) + op.alter_column( + "pages", + "repository_sidecar_extension", + existing_type=sa.VARCHAR(length=255), + type_=sa.UnicodeText(), + existing_nullable=True, + ) + + +def downgrade() -> None: + op.alter_column( + "pages", + "repository_sidecar_extension", + existing_type=sa.UnicodeText(), + type_=sa.VARCHAR(length=255), + existing_nullable=True, + ) + op.alter_column( + "pages", + "repository_source_extension", + existing_type=sa.UnicodeText(), + type_=sa.VARCHAR(length=255), + existing_nullable=True, + ) + op.alter_column( + "pages", + "repository_path_stem", + existing_type=sa.UnicodeText(), + type_=sa.VARCHAR(length=255), + existing_nullable=True, + ) + op.alter_column( + "pages", + "repository_display_path_prefix", + existing_type=sa.UnicodeText(), + type_=sa.VARCHAR(length=2048), + existing_nullable=True, + ) + op.alter_column( + "pages", + "repository_path_prefix", + existing_type=sa.UnicodeText(), + type_=sa.VARCHAR(length=2048), + existing_nullable=True, + ) + op.alter_column( + "pages", + "github_repo", + existing_type=sa.UnicodeText(), + type_=sa.VARCHAR(length=255), + existing_nullable=True, + ) + op.alter_column( + "pages", + "github_owner", + existing_type=sa.UnicodeText(), + type_=sa.VARCHAR(length=255), + existing_nullable=True, + ) + op.alter_column( + "pages", + "uploader_username", + existing_type=sa.UnicodeText(), + type_=sa.VARCHAR(length=64), + existing_nullable=True, + ) diff --git a/src/timessquare/dbschema/page.py b/src/timessquare/dbschema/page.py index 4927874..b0c7230 100644 --- a/src/timessquare/dbschema/page.py +++ b/src/timessquare/dbschema/page.py @@ -60,7 +60,7 @@ class SqlPage(Base): """Tags (keywords) assigned to this page.""" uploader_username: Mapped[str | None] = mapped_column( - Unicode(64), nullable=True + UnicodeText, nullable=True ) """Username of the uploader, if this page is uploaded without GitHub backing. @@ -79,12 +79,12 @@ class SqlPage(Base): indefinitely. """ - github_owner: Mapped[str | None] = mapped_column(Unicode(255)) + github_owner: Mapped[str | None] = mapped_column(UnicodeText) """The GitHub repository owner (username or organization name) for GitHub-backed pages. """ - github_repo: Mapped[str | None] = mapped_column(Unicode(255)) + github_repo: Mapped[str | None] = mapped_column(UnicodeText) """The GitHub repository name for GitHub-backed pages.""" github_commit: Mapped[str | None] = mapped_column(Unicode(40)) @@ -92,17 +92,17 @@ class SqlPage(Base): associated with a GitHub Check Run. """ - repository_path_prefix: Mapped[str | None] = mapped_column(Unicode(2048)) + repository_path_prefix: Mapped[str | None] = mapped_column(UnicodeText) """The repository path prefix, relative to the root of the directory.""" repository_display_path_prefix: Mapped[str | None] = mapped_column( - Unicode(2048) + UnicodeText ) """The repository path prefix, relative to the configured root of Times Square notebooks in a repository. """ - repository_path_stem: Mapped[str | None] = mapped_column(Unicode(255)) + repository_path_stem: Mapped[str | None] = mapped_column(UnicodeText) """The filename stem (without prefix and without extension) of the source file in the GitHub repository for GitHub-backed pages. @@ -112,7 +112,7 @@ class SqlPage(Base): """ repository_source_extension: Mapped[str | None] = mapped_column( - Unicode(255) + UnicodeText ) """The filename extension of the source file in the GitHub repository for GitHub-backed pages. @@ -121,7 +121,7 @@ class SqlPage(Base): """ repository_sidecar_extension: Mapped[str | None] = mapped_column( - Unicode(255) + UnicodeText ) """The filename extension of the sidecar YAML file in the GitHub repository for GitHub-backed pages. From 6b312df311c39f50a6847fbd84756c9892efbf34 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Tue, 14 Jan 2025 11:45:52 -0500 Subject: [PATCH 3/5] Document concrete steps for database migrations The steps for Times Square are slightly different from the generic Safir docs, so we'll provide the exact copy-and-pastable steps here. --- docs/dev/development.rst | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/docs/dev/development.rst b/docs/dev/development.rst index f86e294..97b2282 100644 --- a/docs/dev/development.rst +++ b/docs/dev/development.rst @@ -70,7 +70,44 @@ Database migrations Times Square uses Alembic_ for database migrations. If your work involves changing the database schema (in :file:`/src/timessquare/dbschema`) you will need to prepare an Alembic migration in the same PR. -This process is outlined in the `Safir documentation `__. +To create the migration, you will need to run a local postgres database for Alembic to compare the current schema to the new schema: + +1. With your existing codebase (before your changes; switch branches or stash changes if necessary), start up the database: + + .. code-block:: sh + + docker-compose -f docker-compose.yaml up + +2. Initialize the database: + + .. code-block:: sh + + tox run -e cli -- init + +3. Apply code changes to the database schema in :file:`/src/timessquare/dbschema`. + +4. Generate the Alembic migration: + + .. code-block:: sh + + tox run -e alembic -- revision --autogenerate -m "Your migration message." + +5. Review the migration in :file:`alembic/versions/` and make any necessary changes. + In particular, enums require special handling. See the `Safir documentation `__ for more information. + +6. Apply the migration to the running database: + + .. code-block:: sh + + tox run -e cli -- update-db-schema --alembic-config-path alembic.ini + +7. Shut down the database: + + .. code-block:: sh + + docker-compose -f docker-compose.yaml down + +For more general information about preparing Alembic migrations, see the `Safir documentation `__. Note that in Times Square the :file:`docker-compose.yaml` is hosted in the root of the repository rather than in the :file:`alembic` directory. Building documentation From c9c5cf6bdfc4a87f2d173dbfee4a4f2edc3a0079 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Tue, 14 Jan 2025 11:54:07 -0500 Subject: [PATCH 4/5] Add change log entry for TEXT column migration --- changelog.d/20250114_114726_jsick_DM_48413.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 changelog.d/20250114_114726_jsick_DM_48413.md diff --git a/changelog.d/20250114_114726_jsick_DM_48413.md b/changelog.d/20250114_114726_jsick_DM_48413.md new file mode 100644 index 0000000..5de19d4 --- /dev/null +++ b/changelog.d/20250114_114726_jsick_DM_48413.md @@ -0,0 +1,17 @@ + + +### Backwards-incompatible changes + +- Migrate the database to use `TEXT` column types where previously we used `VARCHAR` columns with a (now unnecessary) length limit. **This change requires a database migration on deployment**. In Postgres there is no functional or performance difference between `VARCHAR` and `TEXT` columns. This change simplifies the database schema and reduce the risk of future issues with column length limits. + +### New features + +- + +### Bug fixes + +- In the `cli` tox environment, fix the name of the executable to be `times-square` rather than `timessquare`. + +### Other changes + +- Improved the developer documentation for database migration to concretely provide copy-and-paste-able commands for preparing and running database migrations. From 5d13bc42f1fe159ebbe94b4f73e484b001dc1e66 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Wed, 15 Jan 2025 15:21:26 -0500 Subject: [PATCH 5/5] Update change log for version 0.15.0 release --- CHANGELOG.md | 16 ++++++++++++++++ changelog.d/20250114_114726_jsick_DM_48413.md | 17 ----------------- 2 files changed, 16 insertions(+), 17 deletions(-) delete mode 100644 changelog.d/20250114_114726_jsick_DM_48413.md diff --git a/CHANGELOG.md b/CHANGELOG.md index cd09926..cd97b7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,22 @@ Collect fragments into this file with: scriv collect --version X.Y.Z + + +## 0.15.0 (2025-01-15) + +### Backwards-incompatible changes + +- Migrate the database to use `TEXT` column types where previously we used `VARCHAR` columns with a (now unnecessary) length limit. **This change requires a database migration on deployment**. In Postgres there is no functional or performance difference between `VARCHAR` and `TEXT` columns. This change simplifies the database schema and reduce the risk of future issues with column length limits. + +### Bug fixes + +- In the `cli` tox environment, fix the name of the executable to be `times-square` rather than `timessquare`. + +### Other changes + +- Improved the developer documentation for database migration to concretely provide copy-and-paste-able commands for preparing and running database migrations. + ## 0.14.0 (2025-01-13) diff --git a/changelog.d/20250114_114726_jsick_DM_48413.md b/changelog.d/20250114_114726_jsick_DM_48413.md deleted file mode 100644 index 5de19d4..0000000 --- a/changelog.d/20250114_114726_jsick_DM_48413.md +++ /dev/null @@ -1,17 +0,0 @@ - - -### Backwards-incompatible changes - -- Migrate the database to use `TEXT` column types where previously we used `VARCHAR` columns with a (now unnecessary) length limit. **This change requires a database migration on deployment**. In Postgres there is no functional or performance difference between `VARCHAR` and `TEXT` columns. This change simplifies the database schema and reduce the risk of future issues with column length limits. - -### New features - -- - -### Bug fixes - -- In the `cli` tox environment, fix the name of the executable to be `times-square` rather than `timessquare`. - -### Other changes - -- Improved the developer documentation for database migration to concretely provide copy-and-paste-able commands for preparing and running database migrations.