sbp commented on code in PR #1376: URL: https://github.com/apache/tooling-trusted-releases/pull/1376#discussion_r3554017843
########## atr/shared/catalogue_diff.py: ########## @@ -0,0 +1,228 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. + +import dataclasses + +import pydantic + +import atr.shared.catalogue_rows as catalogue_rows + + [email protected](frozen=True) +class DbSnapshot: + committee_keys: frozenset[str] + project_committee: dict[str, str] + release_project: dict[str, str] + artifact_by_dist: dict[tuple[str, str], tuple[str, str, str, str | None]] + managed_project_keys: frozenset[str] + # Every release key foundation-wide, unlike the workspace-scoped release_project + release_keys: frozenset[str] = dataclasses.field(default_factory=frozenset) + + [email protected](frozen=True) +class Conflict: + table: str + key: str + reason: str + + [email protected](frozen=True) +class ArtifactRehome: + dist: tuple[str, str] + from_pk: tuple[str, str, str] + to_row: catalogue_rows.ArtifactRow + + [email protected](frozen=True) +class ReleaseMove: + key: str + from_project: str + to_project: str + to_committee: str + cross_committee: bool + row: catalogue_rows.ReleaseRow + + [email protected] +class CatalogueDiff: + adds: list[catalogue_rows.Row] = dataclasses.field(default_factory=list) + artifact_rehomes: list[ArtifactRehome] = dataclasses.field(default_factory=list) + release_moves: list[ReleaseMove] = dataclasses.field(default_factory=list) + conflicts: list[Conflict] = dataclasses.field(default_factory=list) + unchanged: int = 0 + + @property + def counts(self) -> dict[str, int]: + return { + "add": len(self.adds), + "artifact_rehome": len(self.artifact_rehomes), + "release_move": len(self.release_moves), + "conflict": len(self.conflicts), + "unchanged": self.unchanged, + } + + +def classify_catalogue( + rows_by_table: dict[str, list[dict[str, str]]], snapshot: DbSnapshot, committee_key: str +) -> CatalogueDiff: + diff = CatalogueDiff() + projects = _parse(diff, "projects", "key", catalogue_rows.ProjectRow, rows_by_table.get("projects", [])) + releases = _parse(diff, "releases", "key", catalogue_rows.ReleaseRow, rows_by_table.get("releases", [])) + artifacts = _parse( + diff, "artifacts", "artifact_path", catalogue_rows.ArtifactRow, rows_by_table.get("artifacts", []) + ) + _classify_projects(projects, snapshot, diff) + _classify_releases(releases, snapshot, diff, committee_key) + _classify_artifacts(artifacts, snapshot, diff) + return diff + + +def _artifact_reference_conflict( + row: catalogue_rows.ArtifactRow, snapshot: DbSnapshot, known_projects: set[str], known_releases: set[str] +) -> Conflict | None: + path = str(row.artifact_path) + project_key = str(row.project_key) + if project_key in snapshot.managed_project_keys: + return Conflict(table="artifacts", key=path, reason="target project has live workflow data") + if project_key not in known_projects: + return Conflict(table="artifacts", key=path, reason=f"project '{project_key}' does not exist") + release_key = str(row.release_key) if row.release_key else "" + if release_key and (release_key not in known_releases): + return Conflict(table="artifacts", key=path, reason=f"release '{release_key}' does not exist") + return None + + +def _classify_artifacts(rows: list[catalogue_rows.ArtifactRow], snapshot: DbSnapshot, diff: CatalogueDiff) -> None: + # A project or release added earlier in this diff counts as present + known_projects = set(snapshot.project_committee) | { + str(add.key) for add in diff.adds if isinstance(add, catalogue_rows.ProjectRow) + } + known_releases = ( + set(snapshot.release_project) + | {str(add.key) for add in diff.adds if isinstance(add, catalogue_rows.ReleaseRow)} + | {f"{move.to_project}-{move.row.version}" for move in diff.release_moves} + ) + existing_pks = {(pk[0], pk[1], pk[2]) for pk in snapshot.artifact_by_dist.values()} + for row in rows: + reference_conflict = _artifact_reference_conflict(row, snapshot, known_projects, known_releases) + if reference_conflict is not None: + diff.conflicts.append(reference_conflict) + continue + path = str(row.artifact_path) + pk = (str(row.project_key), str(row.version), path) + suffix = str(row.download_path_suffix) if row.download_path_suffix else "" + matched = snapshot.artifact_by_dist.get((suffix, path)) if suffix else None Review Comment: So this is looking up artifacts by `(download path, filename)`? Problem is, since download path is an exported field, it's also editable. If anybody changes it, how will it know what's being moved? So, any download path edit becomes a crash: it's an actual 500. ########## atr/shared/catalogue_diff.py: ########## @@ -0,0 +1,228 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. + +import dataclasses + +import pydantic + +import atr.shared.catalogue_rows as catalogue_rows + + [email protected](frozen=True) +class DbSnapshot: + committee_keys: frozenset[str] + project_committee: dict[str, str] + release_project: dict[str, str] + artifact_by_dist: dict[tuple[str, str], tuple[str, str, str, str | None]] + managed_project_keys: frozenset[str] + # Every release key foundation-wide, unlike the workspace-scoped release_project + release_keys: frozenset[str] = dataclasses.field(default_factory=frozenset) + + [email protected](frozen=True) +class Conflict: + table: str + key: str + reason: str + + [email protected](frozen=True) +class ArtifactRehome: + dist: tuple[str, str] + from_pk: tuple[str, str, str] + to_row: catalogue_rows.ArtifactRow + + [email protected](frozen=True) +class ReleaseMove: + key: str + from_project: str + to_project: str + to_committee: str + cross_committee: bool + row: catalogue_rows.ReleaseRow + + [email protected] +class CatalogueDiff: + adds: list[catalogue_rows.Row] = dataclasses.field(default_factory=list) + artifact_rehomes: list[ArtifactRehome] = dataclasses.field(default_factory=list) + release_moves: list[ReleaseMove] = dataclasses.field(default_factory=list) + conflicts: list[Conflict] = dataclasses.field(default_factory=list) + unchanged: int = 0 + + @property + def counts(self) -> dict[str, int]: + return { + "add": len(self.adds), + "artifact_rehome": len(self.artifact_rehomes), + "release_move": len(self.release_moves), + "conflict": len(self.conflicts), + "unchanged": self.unchanged, + } + + +def classify_catalogue( + rows_by_table: dict[str, list[dict[str, str]]], snapshot: DbSnapshot, committee_key: str +) -> CatalogueDiff: + diff = CatalogueDiff() + projects = _parse(diff, "projects", "key", catalogue_rows.ProjectRow, rows_by_table.get("projects", [])) + releases = _parse(diff, "releases", "key", catalogue_rows.ReleaseRow, rows_by_table.get("releases", [])) + artifacts = _parse( + diff, "artifacts", "artifact_path", catalogue_rows.ArtifactRow, rows_by_table.get("artifacts", []) + ) + _classify_projects(projects, snapshot, diff) + _classify_releases(releases, snapshot, diff, committee_key) + _classify_artifacts(artifacts, snapshot, diff) + return diff + + +def _artifact_reference_conflict( + row: catalogue_rows.ArtifactRow, snapshot: DbSnapshot, known_projects: set[str], known_releases: set[str] +) -> Conflict | None: + path = str(row.artifact_path) + project_key = str(row.project_key) + if project_key in snapshot.managed_project_keys: + return Conflict(table="artifacts", key=path, reason="target project has live workflow data") + if project_key not in known_projects: + return Conflict(table="artifacts", key=path, reason=f"project '{project_key}' does not exist") + release_key = str(row.release_key) if row.release_key else "" + if release_key and (release_key not in known_releases): + return Conflict(table="artifacts", key=path, reason=f"release '{release_key}' does not exist") + return None + + +def _classify_artifacts(rows: list[catalogue_rows.ArtifactRow], snapshot: DbSnapshot, diff: CatalogueDiff) -> None: + # A project or release added earlier in this diff counts as present + known_projects = set(snapshot.project_committee) | { + str(add.key) for add in diff.adds if isinstance(add, catalogue_rows.ProjectRow) + } + known_releases = ( + set(snapshot.release_project) + | {str(add.key) for add in diff.adds if isinstance(add, catalogue_rows.ReleaseRow)} + | {f"{move.to_project}-{move.row.version}" for move in diff.release_moves} + ) + existing_pks = {(pk[0], pk[1], pk[2]) for pk in snapshot.artifact_by_dist.values()} + for row in rows: + reference_conflict = _artifact_reference_conflict(row, snapshot, known_projects, known_releases) + if reference_conflict is not None: + diff.conflicts.append(reference_conflict) + continue + path = str(row.artifact_path) + pk = (str(row.project_key), str(row.version), path) + suffix = str(row.download_path_suffix) if row.download_path_suffix else "" + matched = snapshot.artifact_by_dist.get((suffix, path)) if suffix else None + if matched is not None and (matched[0], matched[1], matched[2]) == pk: + diff.unchanged += 1 + continue + if matched is not None: + if pk in existing_pks: + diff.conflicts.append(Conflict(table="artifacts", key=path, reason="re-home target already exists")) + continue + diff.artifact_rehomes.append( + ArtifactRehome(dist=(suffix, path), from_pk=(matched[0], matched[1], matched[2]), to_row=row) + ) + continue + if not suffix: + diff.conflicts.append( + Conflict(table="artifacts", key=path, reason="unresolvable identity (no dist suffix)") + ) + continue + diff.adds.append(row) + + +def _classify_projects(rows: list[catalogue_rows.ProjectRow], snapshot: DbSnapshot, diff: CatalogueDiff) -> None: + for row in rows: + key = str(row.key) + if key in snapshot.managed_project_keys: + diff.conflicts.append(Conflict(table="projects", key=key, reason="project has live workflow data")) + continue + if key in snapshot.project_committee: + diff.unchanged += 1 + continue + committee = str(row.committee_key) if row.committee_key else "" + if committee not in snapshot.committee_keys: + diff.conflicts.append(Conflict(table="projects", key=key, reason=f"committee '{committee}' does not exist")) + continue + diff.adds.append(row) + + +def _classify_releases( + rows: list[catalogue_rows.ReleaseRow], snapshot: DbSnapshot, diff: CatalogueDiff, committee_key: str +) -> None: + pending = { + str(add.key): (str(add.committee_key) if add.committee_key else None) + for add in diff.adds + if isinstance(add, catalogue_rows.ProjectRow) + } + for row in rows: + key = str(row.key) + target_project = str(row.project_key) + if target_project in snapshot.managed_project_keys: + diff.conflicts.append(Conflict(table="releases", key=key, reason="target project has live workflow data")) + continue + target_committee = snapshot.project_committee.get(target_project) or pending.get(target_project) + if target_committee is None: + diff.conflicts.append( + Conflict(table="releases", key=key, reason=f"target project '{target_project}' does not exist") + ) + continue + current_project = snapshot.release_project.get(key) + if current_project is None: + diff.adds.append(row) Review Comment: I guess `if target_release_key in snapshot.release_keys` was intended to be the global check here (it's below in the code), but because there's a `continue` it's never reached. Basically this check is only looking at whether it's a release key in the existing committee, whereas the later check, which is not reached, would check whether it's a release key in the whole database. Small point, but also easy to fix. ########## atr/storage/writers/catalogue.py: ########## @@ -227,6 +222,111 @@ async def delete_project(self, project_key: safe.ProjectKey) -> None: raise self.__write_as.append_to_audit_log(asf_uid=self.__asf_uid, deleted_project=key) + async def import_catalogue_csvs( + self, rows_by_table: dict[str, list[dict[str, str]]], committee_key: str + ) -> catalogue_diff.CatalogueDiff: + # Recompute the diff against a snapshot taken inside this transaction, under the write lock + await self.__data.begin_immediate() + self.__data.expire_all() + try: + snapshot = await catalogue_import.build_snapshot(self.__data, committee_key) + diff = catalogue_diff.classify_catalogue(rows_by_table, snapshot, committee_key) + await self._apply_diff(diff) + await self.__data.commit() + except Exception: + await self.__data.rollback() + raise + self._audit_import(diff) + return diff + + async def _apply_diff(self, diff: catalogue_diff.CatalogueDiff) -> None: + await self.__data.execute(sqlalchemy.text("PRAGMA defer_foreign_keys=ON")) + # Adds in FK order: projects, then releases, then artifacts + for add in diff.adds: + if isinstance(add, catalogue_rows.ProjectRow): + self.__data.add(add.to_sql()) + await self.__data.flush() + for add in diff.adds: + if isinstance(add, catalogue_rows.ReleaseRow): + self.__data.add(add.to_sql()) + await self.__data.flush() + for add in diff.adds: + if isinstance(add, catalogue_rows.ArtifactRow): + self.__data.add(add.to_sql()) + for move in diff.release_moves: + await self._move_one_release(move) + for rehome in diff.artifact_rehomes: + await self._rehome_one_artifact(rehome) + await self._assert_fk_integrity() + + def _audit_import(self, diff: catalogue_diff.CatalogueDiff) -> None: + counts = diff.counts + self.__write_as.append_to_audit_log( + asf_uid=self.__asf_uid, + added=counts["add"], + moved=counts["release_move"], + rehomed=counts["artifact_rehome"], + conflicts=counts["conflict"], + ) + + async def _move_one_release(self, move: catalogue_diff.ReleaseMove) -> None: + via = sql.validate_instrumented_attribute + old_key = move.key + version = str(move.row.version) + new_key = f"{move.to_project}-{version}" Review Comment: If the uploader gets the version wrong, couldn't this corrupt the release? There doesn't appear to be any validation of this `new_key`. I tested, and if there are artifacts then you get a 500 error anyway, but it breaks the whole import. If there are no artifacts, it corrupts silently. I assume that this will cause problems elsewhere, but I didn't check (I mean, release key is `proj-0.1` but version is `0.2`). ########## atr/storage/writers/catalogue.py: ########## @@ -227,6 +222,111 @@ async def delete_project(self, project_key: safe.ProjectKey) -> None: raise self.__write_as.append_to_audit_log(asf_uid=self.__asf_uid, deleted_project=key) + async def import_catalogue_csvs( + self, rows_by_table: dict[str, list[dict[str, str]]], committee_key: str + ) -> catalogue_diff.CatalogueDiff: + # Recompute the diff against a snapshot taken inside this transaction, under the write lock + await self.__data.begin_immediate() + self.__data.expire_all() + try: + snapshot = await catalogue_import.build_snapshot(self.__data, committee_key) + diff = catalogue_diff.classify_catalogue(rows_by_table, snapshot, committee_key) + await self._apply_diff(diff) + await self.__data.commit() + except Exception: + await self.__data.rollback() + raise + self._audit_import(diff) + return diff + + async def _apply_diff(self, diff: catalogue_diff.CatalogueDiff) -> None: + await self.__data.execute(sqlalchemy.text("PRAGMA defer_foreign_keys=ON")) + # Adds in FK order: projects, then releases, then artifacts + for add in diff.adds: + if isinstance(add, catalogue_rows.ProjectRow): + self.__data.add(add.to_sql()) + await self.__data.flush() + for add in diff.adds: + if isinstance(add, catalogue_rows.ReleaseRow): + self.__data.add(add.to_sql()) + await self.__data.flush() + for add in diff.adds: + if isinstance(add, catalogue_rows.ArtifactRow): + self.__data.add(add.to_sql()) + for move in diff.release_moves: + await self._move_one_release(move) + for rehome in diff.artifact_rehomes: + await self._rehome_one_artifact(rehome) + await self._assert_fk_integrity() + + def _audit_import(self, diff: catalogue_diff.CatalogueDiff) -> None: + counts = diff.counts + self.__write_as.append_to_audit_log( + asf_uid=self.__asf_uid, + added=counts["add"], + moved=counts["release_move"], + rehomed=counts["artifact_rehome"], + conflicts=counts["conflict"], + ) + + async def _move_one_release(self, move: catalogue_diff.ReleaseMove) -> None: + via = sql.validate_instrumented_attribute + old_key = move.key + version = str(move.row.version) + new_key = f"{move.to_project}-{version}" + await self.__data.execute( + sqlmodel.update(sql.Artifact) + .where(via(sql.Artifact.project_key) == move.from_project) + .where(via(sql.Artifact.version) == version) + .values(release_key=new_key, project_key=move.to_project) + ) + await self.__data.execute( + sqlmodel.update(sql.LifecycleEvent) + .where(via(sql.LifecycleEvent.version_key) == old_key) + .values(version_key=new_key, project_key=move.to_project) + ) + # Other release-key children re-point to the new key + for model, attr in repoint.RELEASE_KEY_REFS: Review Comment: This contains `sql.Task.version_key`, but it shouldn't do because that's not a release key (`example-0.1`), it's just the version (`0.1`): ``` RELEASE_KEY_REFS: Final[list[tuple[type[sqlmodel.SQLModel], str]]] = [ (sql.Release, "key"), (sql.Artifact, "release_key"), (sql.LifecycleEvent, "version_key"), (sql.CheckResult, "release_key"), (sql.BallotPaper, "release_key"), (sql.Distribution, "release_key"), (sql.Quarantined, "release_key"), (sql.Revision, "release_key"), (sql.Task, "version_key"), ] ``` ########## atr/shared/catalogue_diff.py: ########## @@ -0,0 +1,228 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. + +import dataclasses + +import pydantic + +import atr.shared.catalogue_rows as catalogue_rows + + [email protected](frozen=True) +class DbSnapshot: + committee_keys: frozenset[str] + project_committee: dict[str, str] + release_project: dict[str, str] + artifact_by_dist: dict[tuple[str, str], tuple[str, str, str, str | None]] + managed_project_keys: frozenset[str] + # Every release key foundation-wide, unlike the workspace-scoped release_project + release_keys: frozenset[str] = dataclasses.field(default_factory=frozenset) + + [email protected](frozen=True) +class Conflict: + table: str + key: str + reason: str + + [email protected](frozen=True) +class ArtifactRehome: + dist: tuple[str, str] + from_pk: tuple[str, str, str] + to_row: catalogue_rows.ArtifactRow + + [email protected](frozen=True) +class ReleaseMove: + key: str + from_project: str + to_project: str + to_committee: str + cross_committee: bool + row: catalogue_rows.ReleaseRow + + [email protected] +class CatalogueDiff: + adds: list[catalogue_rows.Row] = dataclasses.field(default_factory=list) + artifact_rehomes: list[ArtifactRehome] = dataclasses.field(default_factory=list) + release_moves: list[ReleaseMove] = dataclasses.field(default_factory=list) + conflicts: list[Conflict] = dataclasses.field(default_factory=list) + unchanged: int = 0 + + @property + def counts(self) -> dict[str, int]: + return { + "add": len(self.adds), + "artifact_rehome": len(self.artifact_rehomes), + "release_move": len(self.release_moves), + "conflict": len(self.conflicts), + "unchanged": self.unchanged, + } + + +def classify_catalogue( + rows_by_table: dict[str, list[dict[str, str]]], snapshot: DbSnapshot, committee_key: str +) -> CatalogueDiff: + diff = CatalogueDiff() + projects = _parse(diff, "projects", "key", catalogue_rows.ProjectRow, rows_by_table.get("projects", [])) + releases = _parse(diff, "releases", "key", catalogue_rows.ReleaseRow, rows_by_table.get("releases", [])) + artifacts = _parse( + diff, "artifacts", "artifact_path", catalogue_rows.ArtifactRow, rows_by_table.get("artifacts", []) + ) + _classify_projects(projects, snapshot, diff) + _classify_releases(releases, snapshot, diff, committee_key) + _classify_artifacts(artifacts, snapshot, diff) + return diff + + +def _artifact_reference_conflict( + row: catalogue_rows.ArtifactRow, snapshot: DbSnapshot, known_projects: set[str], known_releases: set[str] +) -> Conflict | None: + path = str(row.artifact_path) + project_key = str(row.project_key) + if project_key in snapshot.managed_project_keys: + return Conflict(table="artifacts", key=path, reason="target project has live workflow data") + if project_key not in known_projects: + return Conflict(table="artifacts", key=path, reason=f"project '{project_key}' does not exist") + release_key = str(row.release_key) if row.release_key else "" + if release_key and (release_key not in known_releases): + return Conflict(table="artifacts", key=path, reason=f"release '{release_key}' does not exist") + return None + + +def _classify_artifacts(rows: list[catalogue_rows.ArtifactRow], snapshot: DbSnapshot, diff: CatalogueDiff) -> None: + # A project or release added earlier in this diff counts as present + known_projects = set(snapshot.project_committee) | { + str(add.key) for add in diff.adds if isinstance(add, catalogue_rows.ProjectRow) + } + known_releases = ( + set(snapshot.release_project) + | {str(add.key) for add in diff.adds if isinstance(add, catalogue_rows.ReleaseRow)} + | {f"{move.to_project}-{move.row.version}" for move in diff.release_moves} + ) + existing_pks = {(pk[0], pk[1], pk[2]) for pk in snapshot.artifact_by_dist.values()} + for row in rows: + reference_conflict = _artifact_reference_conflict(row, snapshot, known_projects, known_releases) + if reference_conflict is not None: Review Comment: This only seems to check the destination, not the source. ########## atr/shared/catalogue_diff.py: ########## @@ -0,0 +1,228 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. + +import dataclasses + +import pydantic + +import atr.shared.catalogue_rows as catalogue_rows + + [email protected](frozen=True) +class DbSnapshot: + committee_keys: frozenset[str] + project_committee: dict[str, str] + release_project: dict[str, str] + artifact_by_dist: dict[tuple[str, str], tuple[str, str, str, str | None]] + managed_project_keys: frozenset[str] + # Every release key foundation-wide, unlike the workspace-scoped release_project + release_keys: frozenset[str] = dataclasses.field(default_factory=frozenset) + + [email protected](frozen=True) +class Conflict: + table: str + key: str + reason: str + + [email protected](frozen=True) +class ArtifactRehome: + dist: tuple[str, str] + from_pk: tuple[str, str, str] + to_row: catalogue_rows.ArtifactRow + + [email protected](frozen=True) +class ReleaseMove: + key: str + from_project: str + to_project: str + to_committee: str + cross_committee: bool + row: catalogue_rows.ReleaseRow + + [email protected] +class CatalogueDiff: + adds: list[catalogue_rows.Row] = dataclasses.field(default_factory=list) + artifact_rehomes: list[ArtifactRehome] = dataclasses.field(default_factory=list) + release_moves: list[ReleaseMove] = dataclasses.field(default_factory=list) + conflicts: list[Conflict] = dataclasses.field(default_factory=list) + unchanged: int = 0 + + @property + def counts(self) -> dict[str, int]: + return { + "add": len(self.adds), + "artifact_rehome": len(self.artifact_rehomes), + "release_move": len(self.release_moves), + "conflict": len(self.conflicts), + "unchanged": self.unchanged, + } + + +def classify_catalogue( + rows_by_table: dict[str, list[dict[str, str]]], snapshot: DbSnapshot, committee_key: str +) -> CatalogueDiff: + diff = CatalogueDiff() + projects = _parse(diff, "projects", "key", catalogue_rows.ProjectRow, rows_by_table.get("projects", [])) + releases = _parse(diff, "releases", "key", catalogue_rows.ReleaseRow, rows_by_table.get("releases", [])) + artifacts = _parse( + diff, "artifacts", "artifact_path", catalogue_rows.ArtifactRow, rows_by_table.get("artifacts", []) + ) + _classify_projects(projects, snapshot, diff) + _classify_releases(releases, snapshot, diff, committee_key) + _classify_artifacts(artifacts, snapshot, diff) + return diff + + +def _artifact_reference_conflict( + row: catalogue_rows.ArtifactRow, snapshot: DbSnapshot, known_projects: set[str], known_releases: set[str] +) -> Conflict | None: + path = str(row.artifact_path) + project_key = str(row.project_key) + if project_key in snapshot.managed_project_keys: + return Conflict(table="artifacts", key=path, reason="target project has live workflow data") + if project_key not in known_projects: + return Conflict(table="artifacts", key=path, reason=f"project '{project_key}' does not exist") + release_key = str(row.release_key) if row.release_key else "" + if release_key and (release_key not in known_releases): + return Conflict(table="artifacts", key=path, reason=f"release '{release_key}' does not exist") + return None + + +def _classify_artifacts(rows: list[catalogue_rows.ArtifactRow], snapshot: DbSnapshot, diff: CatalogueDiff) -> None: + # A project or release added earlier in this diff counts as present + known_projects = set(snapshot.project_committee) | { + str(add.key) for add in diff.adds if isinstance(add, catalogue_rows.ProjectRow) + } + known_releases = ( + set(snapshot.release_project) + | {str(add.key) for add in diff.adds if isinstance(add, catalogue_rows.ReleaseRow)} + | {f"{move.to_project}-{move.row.version}" for move in diff.release_moves} + ) + existing_pks = {(pk[0], pk[1], pk[2]) for pk in snapshot.artifact_by_dist.values()} + for row in rows: + reference_conflict = _artifact_reference_conflict(row, snapshot, known_projects, known_releases) + if reference_conflict is not None: Review Comment: Below, `if current_project in snapshot.managed_project_keys` checks the source for releases. Artifact sources aren't checked. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
