From 1400ce5ee5e9dc4685b67ad051f544dbbf48e26a Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Wed, 5 Oct 2016 11:42:50 -0700 Subject: [PATCH] Create Files.requires_python column (#1448) Denormalize Release.requires_python into File.requires_python to better enable performant retrieval of that information when fetching a list of files. --- tests/unit/packaging/test_models.py | 14 +++ ...57_add_requires_python_to_release_files.py | 88 +++++++++++++++++++ warehouse/packaging/models.py | 6 ++ 3 files changed, 108 insertions(+) create mode 100644 warehouse/migrations/versions/be4cf6b58557_add_requires_python_to_release_files.py diff --git a/tests/unit/packaging/test_models.py b/tests/unit/packaging/test_models.py index c102db48389f..a3dddb73cb55 100644 --- a/tests/unit/packaging/test_models.py +++ b/tests/unit/packaging/test_models.py @@ -233,6 +233,20 @@ def test_urls(self, db_session, home_page, download_url, project_urls, class TestFile: + def test_requires_python(self, db_session): + """ Attempt to write a File by setting requires_python directly, + which should fail to validate (it should only be set in Release). + """ + with pytest.raises(RuntimeError): + project = DBProjectFactory.create() + release = DBReleaseFactory.create(project=project) + DBFileFactory.create( + release=release, + filename="{}-{}.tar.gz".format(project.name, release.version), + python_version="source", + requires_python="1.0" + ) + def test_compute_paths(self, db_session): project = DBProjectFactory.create() release = DBReleaseFactory.create(project=project) diff --git a/warehouse/migrations/versions/be4cf6b58557_add_requires_python_to_release_files.py b/warehouse/migrations/versions/be4cf6b58557_add_requires_python_to_release_files.py new file mode 100644 index 000000000000..66c49d371593 --- /dev/null +++ b/warehouse/migrations/versions/be4cf6b58557_add_requires_python_to_release_files.py @@ -0,0 +1,88 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Add a requires_python column to release_files; pursuant to enabling PEP 503. + +Revision ID: be4cf6b58557 +Revises: 3d2b8a42219a +Create Date: 2016-09-15 04:12:53.430363 +""" + +from alembic import op +import sqlalchemy as sa + + +revision = 'be4cf6b58557' +down_revision = '3d2b8a42219a' + + +def upgrade(): + """ + Add column `requires_python` in the `release_files` table. + """ + op.add_column("release_files", + sa.Column("requires_python", + sa.Text(), + nullable=True) + ) + + # Populate the column with content from release.requires_python. + op.execute( + """ UPDATE release_files + SET requires_python = releases.requires_python + FROM releases + WHERE + release_files.name=releases.name + AND release_files.version=releases.version; + """ + ) + + # Setup a trigger function to ensure that requires_python value on + # releases is always canonical. + op.execute( + """CREATE OR REPLACE FUNCTION update_release_files_requires_python() + RETURNS TRIGGER AS $$ + BEGIN + UPDATE + release_files + SET + requires_python = releases.requires_python + FROM releases + WHERE + release_files.name=releases.name + AND release_files.version=releases.version + AND release_files.name = NEW.name + AND releases.version = NEW.version; + RETURN NULL; + END; + $$ LANGUAGE plpgsql; + """ + ) + + # Establish a trigger such that on INSERT/UPDATE on releases we update + # release_files with the appropriate requires_python values. + op.execute( + """ CREATE TRIGGER releases_requires_python + AFTER INSERT OR UPDATE OF requires_python ON releases + FOR EACH ROW + EXECUTE PROCEDURE update_release_files_requires_python(); + """ + ) + + +def downgrade(): + """ + Drop trigger and function that synchronize `releases`. + """ + op.execute("DROP TRIGGER releases_requires_python ON releases") + op.execute("DROP FUNCTION update_release_files_requires_python()") + op.drop_column("release_files", "requires_python") diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index 21d5f53ba430..a72f13799ded 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -22,6 +22,7 @@ Boolean, DateTime, Integer, Table, Text, ) from sqlalchemy import func, orm, sql +from sqlalchemy.orm import validates from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.ext.associationproxy import association_proxy from sqlalchemy.ext.declarative import declared_attr @@ -369,6 +370,7 @@ class File(db.Model): name = Column(Text) version = Column(Text) python_version = Column(Text) + requires_python = Column(Text) packagetype = Column( Enum( "bdist_dmg", "bdist_dumb", "bdist_egg", "bdist_msi", "bdist_rpm", @@ -394,6 +396,10 @@ def pgp_path(self): def pgp_path(self): return func.concat(self.path, ".asc") + @validates("requires_python") + def validates_requires_python(self, *args, **kwargs): + raise RuntimeError("Cannot set File.requires_python") + class Filename(db.ModelBase):