From abc8e31b5033cfd383560d278252838064464983 Mon Sep 17 00:00:00 2001
From: sophia <scastellarin@quansight.com>
Date: Wed, 4 Dec 2024 12:32:43 -0800
Subject: [PATCH 01/10] Remove channel column from conda_package_build

This information is duplicated by the package relationship. For
example, the package table already keeps track of the package
channel.
---
 .../conda_store_server/_internal/orm.py          |  9 ---------
 conda-store-server/tests/_internal/test_orm.py   | 16 ++++++++--------
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/conda-store-server/conda_store_server/_internal/orm.py b/conda-store-server/conda_store_server/_internal/orm.py
index 49803941b..3ed157fac 100644
--- a/conda-store-server/conda_store_server/_internal/orm.py
+++ b/conda-store-server/conda_store_server/_internal/orm.py
@@ -756,7 +756,6 @@ class CondaPackageBuild(Base):
 
     __table_args__ = (
         UniqueConstraint(
-            "channel_id",
             "package_id",
             "subdir",
             "build",
@@ -771,14 +770,6 @@ class CondaPackageBuild(Base):
     package_id: Mapped[int] = mapped_column(ForeignKey("conda_package.id"))
     package: Mapped["CondaPackage"] = relationship(back_populates="builds")
 
-    """
-    Some package builds have the exact same data from different channels.
-    Thus, when adding a channel, populating CondaPackageBuild can encounter
-    duplicate keys errors. That's why we need to distinguish them by channel_id.
-    """
-    channel_id: Mapped[int] = mapped_column(ForeignKey("conda_channel.id"))
-    channel: Mapped["CondaChannel"] = relationship(CondaChannel)
-
     build: Mapped[str] = mapped_column(Unicode(64), index=True)
     build_number: Mapped[int]
     constrains: Mapped[dict] = mapped_column(JSON)
diff --git a/conda-store-server/tests/_internal/test_orm.py b/conda-store-server/tests/_internal/test_orm.py
index d35601be2..10eeea25b 100644
--- a/conda-store-server/tests/_internal/test_orm.py
+++ b/conda-store-server/tests/_internal/test_orm.py
@@ -222,7 +222,7 @@ def test_update_packages_add_existing_pkg_new_version(
     assert count == 3
     builds = (
         populated_db.query(orm.CondaPackageBuild)
-        .filter(orm.CondaPackageBuild.channel_id == 1)
+        .filter(orm.CondaPackageBuild.package.channel_id == 1)
         .all()
     )
     assert len(builds) == 4
@@ -297,7 +297,7 @@ def test_update_packages_multiple_subdirs(mock_repdata, populated_db):
     assert count == 2
     builds = (
         populated_db.query(orm.CondaPackageBuild)
-        .filter(orm.CondaPackageBuild.channel_id == 1)
+        .filter(orm.CondaPackageBuild.package.channel_id == 1)
         .all()
     )
     assert len(builds) == 5
@@ -326,13 +326,13 @@ def check_packages():
         assert len(conda_packages) == 1
         conda_packages = (
             populated_db.query(orm.CondaPackageBuild)
-            .filter(orm.CondaPackageBuild.channel_id == 1)
+            .filter(orm.CondaPackageBuild.package.channel_id == 1)
             .all()
         )
         assert len(conda_packages) == 4
         conda_packages = (
             populated_db.query(orm.CondaPackageBuild)
-            .filter(orm.CondaPackageBuild.channel_id == 2)
+            .filter(orm.CondaPackageBuild.package.channel_id == 2)
             .all()
         )
         assert len(conda_packages) == 0
@@ -373,7 +373,7 @@ def test_update_packages_new_package_channel(mock_repdata, populated_db, test_re
     assert count == 2
     builds = (
         populated_db.query(orm.CondaPackageBuild)
-        .filter(orm.CondaPackageBuild.channel_id == 2)
+        .filter(orm.CondaPackageBuild.package.channel_id == 2)
         .all()
     )
     assert len(builds) == 1
@@ -407,7 +407,7 @@ def test_update_packages_multiple_builds(
     assert count == 5
     builds = (
         populated_db.query(orm.CondaPackageBuild)
-        .filter(orm.CondaPackageBuild.channel_id == 2)
+        .filter(orm.CondaPackageBuild.package.channel_id == 2)
         .all()
     )
     assert len(builds) == 2
@@ -429,7 +429,7 @@ def test_update_packages_channel_consistency(
     # ensure the package builds end up with the correct channel
     builds = (
         populated_db.query(orm.CondaPackageBuild)
-        .filter(orm.CondaPackageBuild.channel_id == 2)
+        .filter(orm.CondaPackageBuild.package.channel_id == 2)
         .all()
     )
     for b in builds:
@@ -445,7 +445,7 @@ def test_update_packages_channel_consistency(
     # ensure the package builds end up with the correct channel
     builds = (
         populated_db.query(orm.CondaPackageBuild)
-        .filter(orm.CondaPackageBuild.channel_id == 1)
+        .filter(orm.CondaPackageBuild.package.channel_id == 1)
         .all()
     )
     for b in builds:

From d1fc6c41a3d0008a397414993a63274d3d9bf86b Mon Sep 17 00:00:00 2001
From: sophia <scastellarin@quansight.com>
Date: Wed, 4 Dec 2024 12:46:09 -0800
Subject: [PATCH 02/10] Clean up irrelevant tests

---
 .../tests/_internal/test_orm.py               | 68 +++----------------
 1 file changed, 11 insertions(+), 57 deletions(-)

diff --git a/conda-store-server/tests/_internal/test_orm.py b/conda-store-server/tests/_internal/test_orm.py
index 10eeea25b..a04787182 100644
--- a/conda-store-server/tests/_internal/test_orm.py
+++ b/conda-store-server/tests/_internal/test_orm.py
@@ -220,14 +220,12 @@ def test_update_packages_add_existing_pkg_new_version(
     assert count == 2
     count = populated_db.query(orm.CondaPackage).count()
     assert count == 3
-    builds = (
+    num_builds = (
         populated_db.query(orm.CondaPackageBuild)
         .filter(orm.CondaPackageBuild.package.channel_id == 1)
         .all()
     )
-    assert len(builds) == 4
-    for b in builds:
-        assert b.package.channel_id == 1
+    assert num_builds == 4
     count = populated_db.query(orm.CondaPackageBuild).count()
     assert count == 4
 
@@ -295,14 +293,12 @@ def test_update_packages_multiple_subdirs(mock_repdata, populated_db):
         .count()
     )
     assert count == 2
-    builds = (
+    num_builds = (
         populated_db.query(orm.CondaPackageBuild)
         .filter(orm.CondaPackageBuild.package.channel_id == 1)
-        .all()
+        .count()
     )
-    assert len(builds) == 5
-    for b in builds:
-        assert b.package.channel_id == 1
+    assert num_builds == 5
 
 
 @mock.patch("conda_store_server._internal.conda_utils.download_repodata")
@@ -371,14 +367,12 @@ def test_update_packages_new_package_channel(mock_repdata, populated_db, test_re
         .count()
     )
     assert count == 2
-    builds = (
+    num_builds = (
         populated_db.query(orm.CondaPackageBuild)
         .filter(orm.CondaPackageBuild.package.channel_id == 2)
-        .all()
+        .count()
     )
-    assert len(builds) == 1
-    for b in builds:
-        assert b.package.channel_id == 2
+    assert num_builds == 1
     count = populated_db.query(orm.CondaPackageBuild).count()
     assert count == 4
 
@@ -405,49 +399,9 @@ def test_update_packages_multiple_builds(
     # ensure it is added to conda package builds
     count = populated_db.query(orm.CondaPackageBuild).count()
     assert count == 5
-    builds = (
-        populated_db.query(orm.CondaPackageBuild)
-        .filter(orm.CondaPackageBuild.package.channel_id == 2)
-        .all()
-    )
-    assert len(builds) == 2
-    for b in builds:
-        assert b.package.channel_id == 2
-
-
-@mock.patch("conda_store_server._internal.conda_utils.download_repodata")
-def test_update_packages_channel_consistency(
-    mock_repdata, populated_db, test_repodata_multiple_packages
-):
-    mock_repdata.return_value = test_repodata_multiple_packages
-
-    channel = (
-        populated_db.query(orm.CondaChannel).filter(orm.CondaChannel.id == 2).first()
-    )
-    channel.update_packages(populated_db, "linux-64")
-
-    # ensure the package builds end up with the correct channel
-    builds = (
+    num_builds = (
         populated_db.query(orm.CondaPackageBuild)
         .filter(orm.CondaPackageBuild.package.channel_id == 2)
-        .all()
-    )
-    for b in builds:
-        assert b.channel_id == 2
-        assert b.package.channel_id == 2
-
-    # again with another channel
-    channel = (
-        populated_db.query(orm.CondaChannel).filter(orm.CondaChannel.id == 1).first()
-    )
-    channel.update_packages(populated_db, "linux-64")
-
-    # ensure the package builds end up with the correct channel
-    builds = (
-        populated_db.query(orm.CondaPackageBuild)
-        .filter(orm.CondaPackageBuild.package.channel_id == 1)
-        .all()
+        .count()
     )
-    for b in builds:
-        assert b.channel_id == 1
-        assert b.package.channel_id == 1
+    assert num_builds == 2

From dfb42b3fee8f8ebf1b33715b52da01de33a8887c Mon Sep 17 00:00:00 2001
From: sophia <scastellarin@quansight.com>
Date: Wed, 4 Dec 2024 13:10:06 -0800
Subject: [PATCH 03/10] Update docs for creating a new alembic migration

---
 docusaurus-docs/conda-store/references/database.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docusaurus-docs/conda-store/references/database.md b/docusaurus-docs/conda-store/references/database.md
index d183b16fe..9c91e1f37 100644
--- a/docusaurus-docs/conda-store/references/database.md
+++ b/docusaurus-docs/conda-store/references/database.md
@@ -46,7 +46,7 @@ conda-store relies on [SQLAlchemy](https://www.sqlalchemy.org/) for ORM mapping,
 The procedure to modify the database is the following :
 
 - First, modify [the ORM Model](https://github.com/conda-incubator/conda-store/blob/main/conda-store-server/conda_store_server/orm.py) according to the changes you want to make
-- edit the file `conda-store-server/alembic.ini` and replace the value for entry `sqlalchemy.url` to match the connection URL of your database.
+- edit the file `conda-store-server/conda_store_server/_internal/alembic.ini` and replace the value for entry `sqlalchemy.url` to match the connection URL of your database.
 
 For example (when postgres was started via Docker compose):
 ```
@@ -57,7 +57,7 @@ sqlalchemy.url = postgresql+psycopg2://postgres:password@localhost:5432/conda-st
 - in your command line, run the following :
 
 ```sh
-cd conda-store-server/conda_store_server
+cd conda-store-server/conda_store_server/_internal
 alembic revision --autogenerate -m "description of your changes"
 ```
 

From 29edca0abf58d61b30462167ebf064ceda472ae7 Mon Sep 17 00:00:00 2001
From: sophia <scastellarin@quansight.com>
Date: Wed, 4 Dec 2024 13:10:30 -0800
Subject: [PATCH 04/10] Use python style comments in alembic template

---
 .../conda_store_server/_internal/alembic/script.py.mako     | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/conda-store-server/conda_store_server/_internal/alembic/script.py.mako b/conda-store-server/conda_store_server/_internal/alembic/script.py.mako
index ac4bee237..34b973937 100644
--- a/conda-store-server/conda_store_server/_internal/alembic/script.py.mako
+++ b/conda-store-server/conda_store_server/_internal/alembic/script.py.mako
@@ -1,6 +1,6 @@
-// Copyright (c) conda-store development team. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
+# Copyright (c) conda-store development team. All rights reserved.
+# Use of this source code is governed by a BSD-style
+# license that can be found in the LICENSE file.
 
 """${message}
 

From 464b12c17be2a071b7bda8c3e76388011c79bfbb Mon Sep 17 00:00:00 2001
From: sophia <scastellarin@quansight.com>
Date: Wed, 4 Dec 2024 17:38:59 -0800
Subject: [PATCH 05/10] Write migration for removing redundant channel_id

This will:
  * fix bad db entries
  * remove channel id from conda_package_build table
---
 ...6129_remove_conda_package_build_channel.py | 155 ++++++++++++++++++
 1 file changed, 155 insertions(+)
 create mode 100644 conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py

diff --git a/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py b/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py
new file mode 100644
index 000000000..ebf49c212
--- /dev/null
+++ b/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py
@@ -0,0 +1,155 @@
+# Copyright (c) conda-store development team. All rights reserved.
+# Use of this source code is governed by a BSD-style
+# license that can be found in the LICENSE file.
+
+"""remove conda package build channel
+
+Revision ID: 89637f546129
+Revises: bf065abf375b
+Create Date: 2024-12-04 13:09:25.562450
+
+"""
+from alembic import op, context
+from sqlalchemy import Column, INTEGER, String, ForeignKey, table, select, engine_from_config, pool
+
+# revision identifiers, used by Alembic.
+revision = '89637f546129'
+down_revision = 'bf065abf375b'
+branch_labels = None
+depends_on = None
+
+
+# This function will go thru all the conda_package_build entries and ensure
+# that the right package_id is associated with it
+def fix_misrepresented_packages(conn):
+    # conda_packages is a hash of channel-id_name_version to conda_package id
+    conda_packages = {}
+
+    conda_package_build_table = table(
+        "conda_package_build",
+        Column("id", INTEGER),
+        Column("channel_id", INTEGER),
+        Column("package_id", INTEGER, ForeignKey("conda_package.id")),
+    )
+    conda_package_table = table(
+        "conda_package",
+        Column("id", INTEGER),
+        Column("channel_id", INTEGER),
+        Column("name", String),
+        Column("version", String),
+    )
+
+    def get_conda_package_id(conn, channel_id, name, version):
+        hashed_name = f"{channel_id}-{name}-{version}"
+        
+        # if package exists in the conda_packages dict, return it
+        if hashed_name in conda_packages:
+            return conda_packages[hashed_name]
+        
+        # if not, then query the db for the package
+        package = conn.execute(
+            select(conda_package_table).where(
+                conda_package_table.c.channel_id == channel_id,
+                conda_package_table.c.name == name,
+                conda_package_table.c.version == version,
+            )
+        ).first()
+
+        # save the package into the conda_packages dict
+        conda_packages[hashed_name] = package.id
+        return package.id
+
+    for row in conn.execute(
+        select(
+            conda_package_build_table.c.id,
+            conda_package_build_table.c.package_id,
+            conda_package_build_table.c.channel_id,
+            conda_package_table.c.name,
+            conda_package_table.c.version
+        ).join(
+            conda_package_build_table,
+            conda_package_build_table.c.package_id == conda_package_table.c.id
+        )
+    ):
+        # the channel_id might already be empty
+        if row[2] is None:
+            continue
+        
+        package_id = get_conda_package_id(conn, row[2], row[3], row[4])
+        # if found package id does not match the found package id, we'll need to updated it
+        if package_id != row[1]:
+            update_package_query = conda_package_build_table.update().where(
+                conda_package_build_table.c.id == op.inline_literal(row[0])
+            ).values(
+                {"package_id": op.inline_literal(package_id)}
+            )
+            conn.execute(update_package_query)
+            conn.commit()
+
+def upgrade():
+    target_table = "conda_package_build"
+    bind = op.get_bind()
+
+    # Due to the issue fixed in https://github.com/conda-incubator/conda-store/pull/961
+    # many conda_package_build entries have the wrong package entry (but the right channel).
+    # Because the packages are duplicated, we can not recreate the _conda_package_build_uc
+    # constraint without the channel_id. 
+    # So, go thru each conda_package_build and re-associate it with the correct conda_package
+    # based on the channel id.
+    fix_misrepresented_packages(bind)
+
+    # remove channel column from constraints
+    op.drop_constraint(
+        constraint_name="_conda_package_build_uc",
+        table_name=target_table,
+    )
+
+    # re-add the constraint without the channel column
+    op.create_unique_constraint(
+        constraint_name="_conda_package_build_uc",
+        table_name=target_table,
+        columns=[ 
+            "package_id",
+            "subdir",
+            "build",
+            "build_number",
+            "sha256",
+        ],
+    )
+
+    # remove channel column
+    op.drop_column(
+        target_table, 
+        "channel_id",
+        mssql_drop_foreign_key=True,
+    )
+
+
+def downgrade():
+    target_table = "conda_package_build"
+
+    # remove channel column from constraints
+    op.drop_constraint(
+        constraint_name="_conda_package_build_uc",
+        table_name=target_table,
+    )
+
+    # add channel column
+    op.add_column(
+        target_table,
+        Column("channel_id", INTEGER, ForeignKey("conda_channel.id"))
+    )
+
+    # re-add the constraint with the channel column
+    op.create_unique_constraint(
+        constraint_name="_conda_package_build_uc",
+        table_name=target_table,
+        columns=[ 
+            "channel_id",
+            "package_id",
+            "subdir",
+            "build",
+            "build_number",
+            "sha256",
+        ],
+    )

From 8562a8dee45b1f3384535bb9fab2455042345b29 Mon Sep 17 00:00:00 2001
From: sophia <scastellarin@quansight.com>
Date: Thu, 5 Dec 2024 12:12:25 -0800
Subject: [PATCH 06/10] Check for existence of channel_id

---
 ...6129_remove_conda_package_build_channel.py | 47 +++++++++++--------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py b/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py
index ebf49c212..91f41abca 100644
--- a/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py
+++ b/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py
@@ -9,8 +9,9 @@
 Create Date: 2024-12-04 13:09:25.562450
 
 """
-from alembic import op, context
-from sqlalchemy import Column, INTEGER, String, ForeignKey, table, select, engine_from_config, pool
+from alembic import op
+from sqlalchemy import Column, INTEGER, String, ForeignKey, table, select, inspect
+
 
 # revision identifiers, used by Alembic.
 revision = '89637f546129'
@@ -90,6 +91,12 @@ def upgrade():
     target_table = "conda_package_build"
     bind = op.get_bind()
 
+    # If the channel_id column does not exist, then exit quickly
+    insp = inspect(bind)
+    columns = insp.get_columns(target_table)
+    if "channel_id" not in columns:
+        return
+
     # Due to the issue fixed in https://github.com/conda-incubator/conda-store/pull/961
     # many conda_package_build entries have the wrong package entry (but the right channel).
     # Because the packages are duplicated, we can not recreate the _conda_package_build_uc
@@ -98,24 +105,26 @@ def upgrade():
     # based on the channel id.
     fix_misrepresented_packages(bind)
 
-    # remove channel column from constraints
-    op.drop_constraint(
-        constraint_name="_conda_package_build_uc",
-        table_name=target_table,
-    )
+    # sqlite does not support altering tables
+    if bind.engine.name != "sqlite":
+        # remove channel column from constraints
+        op.drop_constraint(
+            constraint_name="_conda_package_build_uc",
+            table_name=target_table,
+        )
 
-    # re-add the constraint without the channel column
-    op.create_unique_constraint(
-        constraint_name="_conda_package_build_uc",
-        table_name=target_table,
-        columns=[ 
-            "package_id",
-            "subdir",
-            "build",
-            "build_number",
-            "sha256",
-        ],
-    )
+        # re-add the constraint without the channel column
+        op.create_unique_constraint(
+            constraint_name="_conda_package_build_uc",
+            table_name=target_table,
+            columns=[ 
+                "package_id",
+                "subdir",
+                "build",
+                "build_number",
+                "sha256",
+            ],
+        )
 
     # remove channel column
     op.drop_column(

From 5b9b8184e602c9775ef9377e5ed80b544873f604 Mon Sep 17 00:00:00 2001
From: sophia <scastellarin@quansight.com>
Date: Thu, 5 Dec 2024 13:28:21 -0800
Subject: [PATCH 07/10] Fix tests

---
 .../tests/_internal/test_orm.py               | 23 +++++++++++--------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/conda-store-server/tests/_internal/test_orm.py b/conda-store-server/tests/_internal/test_orm.py
index a04787182..ac2e45a73 100644
--- a/conda-store-server/tests/_internal/test_orm.py
+++ b/conda-store-server/tests/_internal/test_orm.py
@@ -53,7 +53,6 @@ def populated_db(db):
             1,
             {
                 "build": "py310h06a4308_0",
-                "channel_id": 1,
                 "build_number": 0,
                 "sha256": "11f080b53b36c056dbd86ccd6dc56c40e3e70359f64279b1658bb69f91ae726f",
                 "subdir": "linux-64",
@@ -68,7 +67,6 @@ def populated_db(db):
             1,
             {
                 "build": "py311h06a4308_0",
-                "channel_id": 1,
                 "build_number": 0,
                 "sha256": "f0719ee6940402a1ea586921acfaf752fda977dbbba74407856a71ba7f6c4e4a",
                 "subdir": "linux-64",
@@ -83,7 +81,6 @@ def populated_db(db):
             1,
             {
                 "build": "py38h06a4308_0",
-                "channel_id": 1,
                 "build_number": 0,
                 "sha256": "39e39a23baebd0598c1b59ae0e82b5ffd6a3230325da4c331231d55cbcf13b3e",
                 "subdir": "linux-64",
@@ -222,8 +219,9 @@ def test_update_packages_add_existing_pkg_new_version(
     assert count == 3
     num_builds = (
         populated_db.query(orm.CondaPackageBuild)
-        .filter(orm.CondaPackageBuild.package.channel_id == 1)
-        .all()
+        .join(orm.CondaPackage)
+        .filter(orm.CondaPackage.channel_id == 1)
+        .count()
     )
     assert num_builds == 4
     count = populated_db.query(orm.CondaPackageBuild).count()
@@ -295,7 +293,8 @@ def test_update_packages_multiple_subdirs(mock_repdata, populated_db):
     assert count == 2
     num_builds = (
         populated_db.query(orm.CondaPackageBuild)
-        .filter(orm.CondaPackageBuild.package.channel_id == 1)
+        .join(orm.CondaPackage)
+        .filter(orm.CondaPackage.channel_id == 1)
         .count()
     )
     assert num_builds == 5
@@ -322,13 +321,15 @@ def check_packages():
         assert len(conda_packages) == 1
         conda_packages = (
             populated_db.query(orm.CondaPackageBuild)
-            .filter(orm.CondaPackageBuild.package.channel_id == 1)
+            .join(orm.CondaPackage)
+            .filter(orm.CondaPackage.channel_id == 1)
             .all()
         )
         assert len(conda_packages) == 4
         conda_packages = (
             populated_db.query(orm.CondaPackageBuild)
-            .filter(orm.CondaPackageBuild.package.channel_id == 2)
+            .join(orm.CondaPackage)
+            .filter(orm.CondaPackage.channel_id == 2)
             .all()
         )
         assert len(conda_packages) == 0
@@ -369,7 +370,8 @@ def test_update_packages_new_package_channel(mock_repdata, populated_db, test_re
     assert count == 2
     num_builds = (
         populated_db.query(orm.CondaPackageBuild)
-        .filter(orm.CondaPackageBuild.package.channel_id == 2)
+        .join(orm.CondaPackage)
+        .filter(orm.CondaPackage.channel_id == 2)
         .count()
     )
     assert num_builds == 1
@@ -401,7 +403,8 @@ def test_update_packages_multiple_builds(
     assert count == 5
     num_builds = (
         populated_db.query(orm.CondaPackageBuild)
-        .filter(orm.CondaPackageBuild.package.channel_id == 2)
+        .join(orm.CondaPackage)
+        .filter(orm.CondaPackage.channel_id == 2)
         .count()
     )
     assert num_builds == 2

From e50082f273700a1feaebb9f4e394f1f3479f2a95 Mon Sep 17 00:00:00 2001
From: sophia <scastellarin@quansight.com>
Date: Thu, 5 Dec 2024 14:51:40 -0800
Subject: [PATCH 08/10] Add dependency on pytest alembic plugin

---
 conda-store-server/environment-dev.yaml | 1 +
 conda-store-server/pyproject.toml       | 1 +
 2 files changed, 2 insertions(+)

diff --git a/conda-store-server/environment-dev.yaml b/conda-store-server/environment-dev.yaml
index 974f0465a..f7fa54a2e 100644
--- a/conda-store-server/environment-dev.yaml
+++ b/conda-store-server/environment-dev.yaml
@@ -22,6 +22,7 @@ dependencies:
   - pytest-celery
   - pytest-mock
   - pytest-cov
+  - pytest-alembic
   - docker-py<=7
   - docker-compose
   # build dependencies
diff --git a/conda-store-server/pyproject.toml b/conda-store-server/pyproject.toml
index cc24b1c1f..8db52c10f 100644
--- a/conda-store-server/pyproject.toml
+++ b/conda-store-server/pyproject.toml
@@ -98,6 +98,7 @@ dependencies = [
   "pytest",
   "pytest-celery",
   "pytest-playwright",
+  "pytest-alembic",
   "twine>=5.0.0",
   "pkginfo>=1.10", # Needed to support metadata 2.3
   "pytest-cov",

From ebe9c2108fea00c1d05cff3e9fc896adbd619f14 Mon Sep 17 00:00:00 2001
From: sophia <scastellarin@quansight.com>
Date: Thu, 5 Dec 2024 14:52:08 -0800
Subject: [PATCH 09/10] Add tests for migration

---
 .gitignore                                    |   3 +
 ...6129_remove_conda_package_build_channel.py |  94 +++++-----
 .../tests/_internal/alembic/__init__.py       |   3 +
 .../_internal/alembic/version/__init__.py     |   3 +
 ...6129_remove_conda_package_build_channel.py | 167 ++++++++++++++++++
 conda-store-server/tests/conftest.py          |  13 ++
 6 files changed, 229 insertions(+), 54 deletions(-)
 create mode 100644 conda-store-server/tests/_internal/alembic/__init__.py
 create mode 100644 conda-store-server/tests/_internal/alembic/version/__init__.py
 create mode 100644 conda-store-server/tests/_internal/alembic/version/test_89637f546129_remove_conda_package_build_channel.py

diff --git a/.gitignore b/.gitignore
index 0ec82f551..75e7762e7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -62,3 +62,6 @@ yarn-error.log*
 conda-store.sqlite
 
 *.lockb
+
+# generated test assets
+conda-store-server/tests/alembic.ini
diff --git a/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py b/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py
index 91f41abca..d8272d8e1 100644
--- a/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py
+++ b/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py
@@ -10,7 +10,7 @@
 
 """
 from alembic import op
-from sqlalchemy import Column, INTEGER, String, ForeignKey, table, select, inspect
+from sqlalchemy import Column, INTEGER, String, ForeignKey, table, select
 
 
 # revision identifiers, used by Alembic.
@@ -19,13 +19,17 @@
 branch_labels = None
 depends_on = None
 
-
-# This function will go thru all the conda_package_build entries and ensure
-# that the right package_id is associated with it
+# Due to the issue fixed in https://github.com/conda-incubator/conda-store/pull/961
+# many conda_package_build entries have the wrong package entry (but the right channel).
+# Because the packages are duplicated, we can not recreate the _conda_package_build_uc
+# constraint without the channel_id. 
+# So, this function will go thru each conda_package_build and re-associate it with the 
+# correct conda_package based on the channel id.
 def fix_misrepresented_packages(conn):
     # conda_packages is a hash of channel-id_name_version to conda_package id
     conda_packages = {}
 
+    # dummy tables to run queries against
     conda_package_build_table = table(
         "conda_package_build",
         Column("id", INTEGER),
@@ -88,36 +92,22 @@ def get_conda_package_id(conn, channel_id, name, version):
             conn.commit()
 
 def upgrade():
-    target_table = "conda_package_build"
     bind = op.get_bind()
 
-    # If the channel_id column does not exist, then exit quickly
-    insp = inspect(bind)
-    columns = insp.get_columns(target_table)
-    if "channel_id" not in columns:
-        return
-
-    # Due to the issue fixed in https://github.com/conda-incubator/conda-store/pull/961
-    # many conda_package_build entries have the wrong package entry (but the right channel).
-    # Because the packages are duplicated, we can not recreate the _conda_package_build_uc
-    # constraint without the channel_id. 
     # So, go thru each conda_package_build and re-associate it with the correct conda_package
     # based on the channel id.
     fix_misrepresented_packages(bind)
 
-    # sqlite does not support altering tables
-    if bind.engine.name != "sqlite":
+    with op.batch_alter_table("conda_package_build") as batch_op:
         # remove channel column from constraints
-        op.drop_constraint(
-            constraint_name="_conda_package_build_uc",
-            table_name=target_table,
+        batch_op.drop_constraint(
+            "_conda_package_build_uc",
         )
 
         # re-add the constraint without the channel column
-        op.create_unique_constraint(
-            constraint_name="_conda_package_build_uc",
-            table_name=target_table,
-            columns=[ 
+        batch_op.create_unique_constraint(
+            "_conda_package_build_uc",
+            [ 
                 "package_id",
                 "subdir",
                 "build",
@@ -126,39 +116,35 @@ def upgrade():
             ],
         )
 
-    # remove channel column
-    op.drop_column(
-        target_table, 
-        "channel_id",
-        mssql_drop_foreign_key=True,
-    )
+        # remove channel column
+        batch_op.drop_column(
+            "channel_id",
+        )
 
 
 def downgrade():
-    target_table = "conda_package_build"
+    with op.batch_alter_table("conda_package_build") as batch_op:
+        # remove channel column from constraints
+        batch_op.drop_constraint(
+            constraint_name="_conda_package_build_uc",
+        )
 
-    # remove channel column from constraints
-    op.drop_constraint(
-        constraint_name="_conda_package_build_uc",
-        table_name=target_table,
-    )
+        # add channel column
+        batch_op.add_column(
+            Column("channel_id", INTEGER)
+        )
 
-    # add channel column
-    op.add_column(
-        target_table,
-        Column("channel_id", INTEGER, ForeignKey("conda_channel.id"))
-    )
+        batch_op.create_foreign_key("fk_channel_id", "conda_channel", ["channel_id"], ["id"])
 
-    # re-add the constraint with the channel column
-    op.create_unique_constraint(
-        constraint_name="_conda_package_build_uc",
-        table_name=target_table,
-        columns=[ 
-            "channel_id",
-            "package_id",
-            "subdir",
-            "build",
-            "build_number",
-            "sha256",
-        ],
-    )
+        # re-add the constraint with the channel column
+        batch_op.create_unique_constraint(
+            constraint_name="_conda_package_build_uc",
+            columns=[ 
+                "channel_id",
+                "package_id",
+                "subdir",
+                "build",
+                "build_number",
+                "sha256",
+            ],
+        )
diff --git a/conda-store-server/tests/_internal/alembic/__init__.py b/conda-store-server/tests/_internal/alembic/__init__.py
new file mode 100644
index 000000000..b559bd2ff
--- /dev/null
+++ b/conda-store-server/tests/_internal/alembic/__init__.py
@@ -0,0 +1,3 @@
+# Copyright (c) conda-store development team. All rights reserved.
+# Use of this source code is governed by a BSD-style
+# license that can be found in the LICENSE file.
diff --git a/conda-store-server/tests/_internal/alembic/version/__init__.py b/conda-store-server/tests/_internal/alembic/version/__init__.py
new file mode 100644
index 000000000..b559bd2ff
--- /dev/null
+++ b/conda-store-server/tests/_internal/alembic/version/__init__.py
@@ -0,0 +1,3 @@
+# Copyright (c) conda-store development team. All rights reserved.
+# Use of this source code is governed by a BSD-style
+# license that can be found in the LICENSE file.
diff --git a/conda-store-server/tests/_internal/alembic/version/test_89637f546129_remove_conda_package_build_channel.py b/conda-store-server/tests/_internal/alembic/version/test_89637f546129_remove_conda_package_build_channel.py
new file mode 100644
index 000000000..ce26b1fc3
--- /dev/null
+++ b/conda-store-server/tests/_internal/alembic/version/test_89637f546129_remove_conda_package_build_channel.py
@@ -0,0 +1,167 @@
+# Copyright (c) conda-store development team. All rights reserved.
+# Use of this source code is governed by a BSD-style
+# license that can be found in the LICENSE file.
+
+from sqlalchemy import insert, select, table, Column, INTEGER, String, ForeignKey, text
+
+from conda_store_server import api
+from conda_store_server._internal import orm
+
+
+def setup_bad_data_db(conda_store):
+    """A database fixture populated with
+    * 2 channels
+    * 2 conda packages
+    * 5 conda package builds
+    """
+    with conda_store.session_factory() as db:
+        # create test channels
+        api.create_conda_channel(db, "test-channel-1")
+        api.create_conda_channel(db, "test-channel-2")
+        db.commit()
+
+        # create some sample conda_package's
+        # For simplicity, the channel_id's match the id of the conda_package.
+        # So, when checking that the package build entries have been reassembled
+        # the right way, check that the package_id in the conda_package_build is
+        # equal to what would have been the channel_id (before the migration is run)
+        conda_package_records = [
+            {
+                "id": 1,
+                "channel_id": 1,
+                "name": "test-package-1",
+                "version": "1.0.0",
+            },
+            {
+                "id": 2,
+                "channel_id": 2,
+                "name": "test-package-1",
+                "version": "1.0.0",
+            },
+        ]
+        for cpb in conda_package_records:
+            conda_package = orm.CondaPackage(**cpb)
+            db.add(conda_package)
+        db.commit()
+
+        # create some conda_package_build's
+        conda_package_builds = [
+            {
+                "id": 1,
+                "build": "py310h06a4308_0",
+                "package_id": 1,
+                "build_number": 0,
+                "sha256": "one",
+                "subdir": "linux-64",
+            },
+            {
+                "id": 2,
+                "build": "py311h06a4308_0",
+                "package_id": 1,
+                "build_number": 0,
+                "sha256": "two",
+                "subdir": "linux-64",
+            },
+            {
+                "id": 3,
+                "build": "py38h06a4308_0",
+                "package_id": 1,
+                "build_number": 0,
+                "sha256": "three",
+                "subdir": "linux-64",
+            },
+            {
+                "id": 4,
+                "build": "py39h06a4308_0",
+                "package_id": 2,
+                "build_number": 0,
+                "sha256": "four",
+                "subdir": "linux-64",
+            },
+            {
+                "id": 5,
+                "build": "py310h06a4308_0",
+                "package_id": 2,
+                "build_number": 0,
+                "sha256": "five",
+                "subdir": "linux-64",
+            },
+        ]
+        default_values = {
+             "depends": "",
+             "md5": "",
+             "timestamp": 0,
+             "constrains": "",
+             "size": 0,
+        }
+        for cpb in conda_package_builds:
+            conda_package = orm.CondaPackageBuild(**cpb, **default_values)
+            db.add(conda_package)
+        db.commit()
+
+        # force in some channel data
+        # conda_package_build 1 should have package_id 2 after migration
+        db.execute(text("UPDATE conda_package_build SET channel_id=2 WHERE id=1"))
+        # conda_package_build 2 should have package_id 1 after migration
+        db.execute(text("UPDATE conda_package_build SET channel_id=1 WHERE id=2"))
+        # conda_package_build 3 should have package_id 1 after migration
+        db.execute(text("UPDATE conda_package_build SET channel_id=1 WHERE id=3"))
+        # conda_package_build 4 should have package_id 2 after migration
+        db.execute(text("UPDATE conda_package_build SET channel_id=2 WHERE id=4"))
+        
+        # don't set conda_package_build 5 channel_id as a test case
+        # conda_package_build 5 package_id should be unchanged (2) after migration
+        
+        db.commit()
+
+def test_remove_conda_package_build_channel_basic(conda_store, alembic_config, alembic_engine, alembic_runner):
+    """Simply run the upgrade and downgrade for this migration"""
+    # migrate all the way to the target revision
+    alembic_runner.migrate_up_to('89637f546129')
+
+    # try downgrading
+    alembic_runner.migrate_down_one()
+
+    # try upgrading once more
+    alembic_runner.migrate_up_one()
+
+def test_remove_conda_package_build_bad_data(conda_store, alembic_config, alembic_engine, alembic_runner):
+    """Simply run the upgrade and downgrade for this migration"""
+     # migrate all the way to the target revision
+    alembic_runner.migrate_up_to('89637f546129')
+    
+    # try downgrading
+    alembic_runner.migrate_down_one()
+
+    # seed db with data that has broken data
+    setup_bad_data_db(conda_store)
+
+    # try upgrading once more
+    alembic_runner.migrate_up_one()
+
+    # ensure all packages builds have the right package associated
+    with conda_store.session_factory() as db:
+        build = db.query(orm.CondaPackageBuild).filter(
+            orm.CondaPackageBuild.id == 1
+        ).first()
+        assert build.package_id == 2
+
+        build = db.query(orm.CondaPackageBuild).filter(
+            orm.CondaPackageBuild.id == 2
+        ).first()
+        assert build.package_id == 1
+
+        build = db.query(orm.CondaPackageBuild).filter(
+            orm.CondaPackageBuild.id == 3
+        ).first()
+        assert build.package_id == 1
+
+        build = db.query(orm.CondaPackageBuild).filter(
+            orm.CondaPackageBuild.id == 4
+        ).first()
+        assert build.package_id == 2
+
+        build = db.query(orm.CondaPackageBuild).filter(
+            orm.CondaPackageBuild.id == 5
+        ).first()
+        assert build.package_id == 2
diff --git a/conda-store-server/tests/conftest.py b/conda-store-server/tests/conftest.py
index f1cebe23b..7b8223c93 100644
--- a/conda-store-server/tests/conftest.py
+++ b/conda-store-server/tests/conftest.py
@@ -13,6 +13,7 @@
 import yaml
 from fastapi.testclient import TestClient
 from sqlalchemy.orm import Session
+from pytest_alembic.config import Config
 
 from conda_store_server import api, app, storage
 
@@ -290,6 +291,18 @@ def plugin_manager():
     return pm
 
 
+@pytest.fixture
+def alembic_config(conda_store):
+     from conda_store_server._internal.dbutil import ALEMBIC_DIR, write_alembic_ini
+     ini_file = pathlib.Path(__file__).parent / "alembic.ini"
+     write_alembic_ini(ini_file, conda_store.database_url)
+     return {"file": ini_file}
+
+@pytest.fixture
+def alembic_engine(db):
+    return db
+
+
 def _seed_conda_store(
     db: Session,
     conda_store,

From 2280bf2e1e407940c120dbe4eb07a857ba28df85 Mon Sep 17 00:00:00 2001
From: "pre-commit-ci[bot]"
 <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date: Mon, 16 Dec 2024 23:18:46 +0000
Subject: [PATCH 10/10] [pre-commit.ci] Apply automatic pre-commit fixes

---
 ...6129_remove_conda_package_build_channel.py | 14 ++--
 ...6129_remove_conda_package_build_channel.py | 74 +++++++++++--------
 conda-store-server/tests/conftest.py          | 11 +--
 3 files changed, 58 insertions(+), 41 deletions(-)

diff --git a/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py b/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py
index d8272d8e1..cfecde382 100644
--- a/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py
+++ b/conda-store-server/conda_store_server/_internal/alembic/versions/89637f546129_remove_conda_package_build_channel.py
@@ -22,8 +22,8 @@
 # Due to the issue fixed in https://github.com/conda-incubator/conda-store/pull/961
 # many conda_package_build entries have the wrong package entry (but the right channel).
 # Because the packages are duplicated, we can not recreate the _conda_package_build_uc
-# constraint without the channel_id. 
-# So, this function will go thru each conda_package_build and re-associate it with the 
+# constraint without the channel_id.
+# So, this function will go thru each conda_package_build and re-associate it with the
 # correct conda_package based on the channel id.
 def fix_misrepresented_packages(conn):
     # conda_packages is a hash of channel-id_name_version to conda_package id
@@ -46,11 +46,11 @@ def fix_misrepresented_packages(conn):
 
     def get_conda_package_id(conn, channel_id, name, version):
         hashed_name = f"{channel_id}-{name}-{version}"
-        
+
         # if package exists in the conda_packages dict, return it
         if hashed_name in conda_packages:
             return conda_packages[hashed_name]
-        
+
         # if not, then query the db for the package
         package = conn.execute(
             select(conda_package_table).where(
@@ -79,7 +79,7 @@ def get_conda_package_id(conn, channel_id, name, version):
         # the channel_id might already be empty
         if row[2] is None:
             continue
-        
+
         package_id = get_conda_package_id(conn, row[2], row[3], row[4])
         # if found package id does not match the found package id, we'll need to updated it
         if package_id != row[1]:
@@ -107,7 +107,7 @@ def upgrade():
         # re-add the constraint without the channel column
         batch_op.create_unique_constraint(
             "_conda_package_build_uc",
-            [ 
+            [
                 "package_id",
                 "subdir",
                 "build",
@@ -139,7 +139,7 @@ def downgrade():
         # re-add the constraint with the channel column
         batch_op.create_unique_constraint(
             constraint_name="_conda_package_build_uc",
-            columns=[ 
+            columns=[
                 "channel_id",
                 "package_id",
                 "subdir",
diff --git a/conda-store-server/tests/_internal/alembic/version/test_89637f546129_remove_conda_package_build_channel.py b/conda-store-server/tests/_internal/alembic/version/test_89637f546129_remove_conda_package_build_channel.py
index ce26b1fc3..12cbe69e1 100644
--- a/conda-store-server/tests/_internal/alembic/version/test_89637f546129_remove_conda_package_build_channel.py
+++ b/conda-store-server/tests/_internal/alembic/version/test_89637f546129_remove_conda_package_build_channel.py
@@ -2,7 +2,7 @@
 # Use of this source code is governed by a BSD-style
 # license that can be found in the LICENSE file.
 
-from sqlalchemy import insert, select, table, Column, INTEGER, String, ForeignKey, text
+from sqlalchemy import text
 
 from conda_store_server import api
 from conda_store_server._internal import orm
@@ -88,11 +88,11 @@ def setup_bad_data_db(conda_store):
             },
         ]
         default_values = {
-             "depends": "",
-             "md5": "",
-             "timestamp": 0,
-             "constrains": "",
-             "size": 0,
+            "depends": "",
+            "md5": "",
+            "timestamp": 0,
+            "constrains": "",
+            "size": 0,
         }
         for cpb in conda_package_builds:
             conda_package = orm.CondaPackageBuild(**cpb, **default_values)
@@ -108,16 +108,19 @@ def setup_bad_data_db(conda_store):
         db.execute(text("UPDATE conda_package_build SET channel_id=1 WHERE id=3"))
         # conda_package_build 4 should have package_id 2 after migration
         db.execute(text("UPDATE conda_package_build SET channel_id=2 WHERE id=4"))
-        
+
         # don't set conda_package_build 5 channel_id as a test case
         # conda_package_build 5 package_id should be unchanged (2) after migration
-        
+
         db.commit()
 
-def test_remove_conda_package_build_channel_basic(conda_store, alembic_config, alembic_engine, alembic_runner):
+
+def test_remove_conda_package_build_channel_basic(
+    conda_store, alembic_config, alembic_engine, alembic_runner
+):
     """Simply run the upgrade and downgrade for this migration"""
     # migrate all the way to the target revision
-    alembic_runner.migrate_up_to('89637f546129')
+    alembic_runner.migrate_up_to("89637f546129")
 
     # try downgrading
     alembic_runner.migrate_down_one()
@@ -125,11 +128,14 @@ def test_remove_conda_package_build_channel_basic(conda_store, alembic_config, a
     # try upgrading once more
     alembic_runner.migrate_up_one()
 
-def test_remove_conda_package_build_bad_data(conda_store, alembic_config, alembic_engine, alembic_runner):
+
+def test_remove_conda_package_build_bad_data(
+    conda_store, alembic_config, alembic_engine, alembic_runner
+):
     """Simply run the upgrade and downgrade for this migration"""
-     # migrate all the way to the target revision
-    alembic_runner.migrate_up_to('89637f546129')
-    
+    # migrate all the way to the target revision
+    alembic_runner.migrate_up_to("89637f546129")
+
     # try downgrading
     alembic_runner.migrate_down_one()
 
@@ -141,27 +147,37 @@ def test_remove_conda_package_build_bad_data(conda_store, alembic_config, alembi
 
     # ensure all packages builds have the right package associated
     with conda_store.session_factory() as db:
-        build = db.query(orm.CondaPackageBuild).filter(
-            orm.CondaPackageBuild.id == 1
-        ).first()
+        build = (
+            db.query(orm.CondaPackageBuild)
+            .filter(orm.CondaPackageBuild.id == 1)
+            .first()
+        )
         assert build.package_id == 2
 
-        build = db.query(orm.CondaPackageBuild).filter(
-            orm.CondaPackageBuild.id == 2
-        ).first()
+        build = (
+            db.query(orm.CondaPackageBuild)
+            .filter(orm.CondaPackageBuild.id == 2)
+            .first()
+        )
         assert build.package_id == 1
 
-        build = db.query(orm.CondaPackageBuild).filter(
-            orm.CondaPackageBuild.id == 3
-        ).first()
+        build = (
+            db.query(orm.CondaPackageBuild)
+            .filter(orm.CondaPackageBuild.id == 3)
+            .first()
+        )
         assert build.package_id == 1
 
-        build = db.query(orm.CondaPackageBuild).filter(
-            orm.CondaPackageBuild.id == 4
-        ).first()
+        build = (
+            db.query(orm.CondaPackageBuild)
+            .filter(orm.CondaPackageBuild.id == 4)
+            .first()
+        )
         assert build.package_id == 2
 
-        build = db.query(orm.CondaPackageBuild).filter(
-            orm.CondaPackageBuild.id == 5
-        ).first()
+        build = (
+            db.query(orm.CondaPackageBuild)
+            .filter(orm.CondaPackageBuild.id == 5)
+            .first()
+        )
         assert build.package_id == 2
diff --git a/conda-store-server/tests/conftest.py b/conda-store-server/tests/conftest.py
index 7b8223c93..929df5965 100644
--- a/conda-store-server/tests/conftest.py
+++ b/conda-store-server/tests/conftest.py
@@ -13,7 +13,6 @@
 import yaml
 from fastapi.testclient import TestClient
 from sqlalchemy.orm import Session
-from pytest_alembic.config import Config
 
 from conda_store_server import api, app, storage
 
@@ -293,10 +292,12 @@ def plugin_manager():
 
 @pytest.fixture
 def alembic_config(conda_store):
-     from conda_store_server._internal.dbutil import ALEMBIC_DIR, write_alembic_ini
-     ini_file = pathlib.Path(__file__).parent / "alembic.ini"
-     write_alembic_ini(ini_file, conda_store.database_url)
-     return {"file": ini_file}
+    from conda_store_server._internal.dbutil import write_alembic_ini
+
+    ini_file = pathlib.Path(__file__).parent / "alembic.ini"
+    write_alembic_ini(ini_file, conda_store.database_url)
+    return {"file": ini_file}
+
 
 @pytest.fixture
 def alembic_engine(db):