Merge pull request #89 from lsst-sqre:tickets/DM-48413

DM-48413: Adopt TEXT column types where useful

CHANGELOG.md

@@ -8,6 +8,22 @@ Collect fragments into this file with: scriv collect --version X.Y.Z
 
 <!-- scriv-insert-here -->
 
+<a id='changelog-0.15.0'></a>
+
+## 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.
+
 <a id='changelog-0.14.0'></a>
 
 ## 0.14.0 (2025-01-13)

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,
+    )

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 <https://safir.lsst.io/user-guide/database/schema.html#testing-database-migrations>`__.
+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 <https://safir.lsst.io/user-guide/database/schema.html#creating-database-migrations>`__ 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 <https://safir.lsst.io/user-guide/database/schema.html#testing-database-migrations>`__.
 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

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,30 +79,30 @@ 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))
     """The SHA of the commit this page corresponds to; only used for pages
     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.

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