Adjust Koji event and model and related methods

These changes are needed for better integration of the implementation
of storing downstream Koji builds in the DB.
Rename build_id to task_id attribute and related methods and make its usage consistent.
Introduce dict attribute of the event rpm_build_task_ids that is a dict of {arch: id}.
Also adjust the method constructing the logs URL to utilise this dictionary.

Fixes packit#2243
Related to packit#1889

packit_service/models.py

@@ -1629,7 +1629,7 @@ def get_all_by_build_id(
         cls, build_id: Union[str, int]
     ) -> Iterable["CoprBuildTargetModel"]:
         if isinstance(build_id, int):
-            # See the comment in get_by_build_id()
+            # See the comment in get_by_task_id()
             build_id = str(build_id)
         return sa_session().query(CoprBuildTargetModel).filter_by(build_id=build_id)
 
@@ -1805,7 +1805,7 @@ class KojiBuildTargetModel(GroupAndTargetModelConnector, Base):
 
     __tablename__ = "koji_build_targets"
     id = Column(Integer, primary_key=True)
-    build_id = Column(String, index=True)  # koji build id
+    task_id = Column(String, index=True)  # ID of the Koji build task
 
     # what's the build status?
     status = Column(String)
@@ -1814,7 +1814,8 @@ class KojiBuildTargetModel(GroupAndTargetModelConnector, Base):
     # URL to koji web ui for the particular build
     web_url = Column(String)
     # url to koji build logs
-    build_logs_url = Column(String)
+    # dictionary with archs and links, e.g. {"x86_64": "my-url"}
+    build_logs_urls = Column(JSON)
     # datetime.utcnow instead of datetime.utcnow() because its an argument to the function
     # so it will run when the koji build is initiated, not when the table is made
     build_submitted_time = Column(DateTime, default=datetime.utcnow)
@@ -1838,19 +1839,19 @@ def set_status(self, status: str):
             self.status = status
             session.add(self)
 
-    def set_build_logs_url(self, build_logs: str):
+    def set_build_logs_urls(self, build_logs: dict):
         with sa_session_transaction() as session:
-            self.build_logs_url = build_logs
+            self.build_logs_urls = build_logs
             session.add(self)
 
     def set_web_url(self, web_url: str):
         with sa_session_transaction() as session:
             self.web_url = web_url
             session.add(self)
 
-    def set_build_id(self, build_id: str):
+    def set_task_id(self, task_id: str):
         with sa_session_transaction() as session:
-            self.build_id = build_id
+            self.task_id = task_id
             session.add(self)
 
     def set_build_start_time(self, build_start_time: Optional[DateTime]):
@@ -1899,39 +1900,27 @@ def get_range(cls, first: int, last: int) -> Iterable["KojiBuildTargetModel"]:
         )
 
     @classmethod
-    def get_all_by_build_id(
-        cls, build_id: Union[str, int]
-    ) -> Iterable["KojiBuildTargetModel"]:
-        """
-        Returns all builds with that build_id, irrespective of target.
-        """
-        if isinstance(build_id, int):
-            # See the comment in get_by_build_id()
-            build_id = str(build_id)
-        return sa_session().query(KojiBuildTargetModel).filter_by(build_id=build_id)
-
-    @classmethod
-    def get_by_build_id(
-        cls, build_id: Union[str, int], target: Optional[str] = None
+    def get_by_task_id(
+        cls, task_id: Union[str, int], target: Optional[str] = None
     ) -> Optional["KojiBuildTargetModel"]:
         """
         Returns the first build matching the build_id and optionally the target.
         """
-        if isinstance(build_id, int):
+        if isinstance(task_id, int):
             # PG is pesky about this:
             #   LINE 3: WHERE koji_builds.build_id = 1245767 AND koji_builds.target ...
             #   HINT:  No operator matches the given name and argument type(s).
             #   You might need to add explicit type casts.
-            build_id = str(build_id)
-        query = sa_session().query(KojiBuildTargetModel).filter_by(build_id=build_id)
+            task_id = str(task_id)
+        query = sa_session().query(KojiBuildTargetModel).filter_by(task_id=task_id)
         if target:
             query = query.filter_by(target=target)
         return query.first()
 
     @classmethod
     def create(
         cls,
-        build_id: Optional[str],
+        task_id: Optional[str],
         web_url: Optional[str],
         target: str,
         status: str,
@@ -1940,7 +1929,7 @@ def create(
     ) -> "KojiBuildTargetModel":
         with sa_session_transaction() as session:
             build = cls()
-            build.build_id = build_id
+            build.task_id = task_id
             build.status = status
             build.web_url = web_url
             build.target = target
@@ -1958,7 +1947,7 @@ def get(
         build_id: str,
         target: str,
     ) -> Optional["KojiBuildTargetModel"]:
-        return cls.get_by_build_id(build_id, target)
+        return cls.get_by_task_id(build_id, target)
 
     def __repr__(self):
         return (
@@ -2911,7 +2900,7 @@ def get_all_by_build_id(
     ) -> Iterable["VMImageBuildTargetModel"]:
         """Returns all builds with that build_id, irrespective of target"""
         if isinstance(build_id, int):
-            # See the comment in get_by_build_id()
+            # See the comment in get_by_task_id()
             build_id = str(build_id)
         return sa_session().query(VMImageBuildTargetModel).filter_by(build_id=build_id)
 

packit_service/service/api/koji_builds.py

@@ -32,13 +32,13 @@ def get(self):
         for build in KojiBuildTargetModel.get_range(first, last):
             build_dict = {
                 "packit_id": build.id,
-                "build_id": build.build_id,
+                "task_id": build.task_id,
                 "status": build.status,
                 "build_submitted_time": optional_timestamp(build.build_submitted_time),
                 "chroot": build.target,
                 "web_url": build.web_url,
                 # from old data, sometimes build_logs_url is same and sometimes different to web_url
-                "build_logs_url": build.build_logs_url,
+                "build_logs_url": build.build_logs_urls,
                 "pr_id": build.get_pr_id(),
                 "branch_name": build.get_branch_name(),
                 "release": build.get_release_tag(),
@@ -77,7 +77,7 @@ def get(self, id):
             )
 
         build_dict = {
-            "build_id": build.build_id,
+            "task_id": build.task_id,
             "status": build.status,
             "chroot": build.target,
             "build_start_time": optional_timestamp(build.build_start_time),
@@ -86,7 +86,7 @@ def get(self, id):
             "commit_sha": build.commit_sha,
             "web_url": build.web_url,
             # from old data, sometimes build_logs_url is same and sometimes different to web_url
-            "build_logs_url": build.build_logs_url,
+            "build_logs_url": build.build_logs_urls,
             "srpm_build_id": build.get_srpm_build().id,
             "run_ids": sorted(run.id for run in build.group_of_targets.runs),
         }

packit_service/service/api/projects.py

@@ -167,7 +167,7 @@ def get(self, forge, namespace, repo_name):
 
             for build in pr.get_koji_builds():
                 build_info = {
-                    "build_id": build.build_id,
+                    "task_id": build.task_id,
                     "chroot": build.target,
                     "status": build.status,
                     "web_url": build.web_url,
@@ -275,7 +275,7 @@ def get(self, forge, namespace, repo_name):
 
             for build in branch.get_koji_builds():
                 build_info = {
-                    "build_id": build.build_id,
+                    "task_id": build.task_id,
                     "chroot": build.target,
                     "status": build.status,
                     "web_url": build.web_url,

packit_service/worker/events/koji.py

@@ -3,21 +3,17 @@
 
 from typing import Union, Optional, Dict
 
-from packit.config import JobConfigTriggerType
-
 from ogr.abstract import GitProject
 from ogr.services.pagure import PagureProject
-
+from packit.config import JobConfigTriggerType
 from packit_service.constants import KojiBuildState, KojiTaskState
 from packit_service.models import (
     AbstractProjectObjectDbType,
     KojiBuildTargetModel,
-    KojiBuildGroupModel,
     PullRequestModel,
     ProjectReleaseModel,
     ProjectEventModel,
     GitBranchModel,
-    PipelineModel,
 )
 from packit_service.worker.events.event import (
     use_for_job_config_trigger,
@@ -27,13 +23,11 @@
 
 class AbstractKojiEvent(AbstractResultEvent):
     def __init__(
-        self,
-        build_id: int,
-        rpm_build_task_id: Optional[int] = None,
+        self, task_id: int, rpm_build_task_ids: Optional[Dict[str, int]] = None
     ):
         super().__init__()
-        self.build_id = build_id
-        self.rpm_build_task_id = rpm_build_task_id
+        self.task_id = task_id
+        self.rpm_build_task_ids = rpm_build_task_ids
 
         # Lazy properties
         self._target: Optional[str] = None
@@ -43,8 +37,8 @@ def __init__(
     @property
     def build_model(self) -> Optional[KojiBuildTargetModel]:
         if not self._build_model_searched and not self._build_model:
-            self._build_model = KojiBuildTargetModel.get_by_build_id(
-                build_id=self.build_id
+            self._build_model = KojiBuildTargetModel.get_by_task_id(
+                task_id=self.task_id
             )
             self._build_model_searched = True
         return self._build_model
@@ -72,6 +66,35 @@ def get_koji_rpm_build_web_url(
         """
         return f"{koji_web_url}/koji/taskinfo?taskID={rpm_build_task_id}"
 
+    @staticmethod
+    def get_koji_build_logs_url(
+        rpm_build_task_id: int,
+        koji_logs_url: str = "https://kojipkgs.fedoraproject.org",
+    ) -> str:
+        """
+        Constructs the log URL for the given Koji task.
+        You can redefine the Koji instance using the one defined in the service config.
+        """
+        return (
+            f"{koji_logs_url}//work/tasks/"
+            f"{rpm_build_task_id % 10000}/{rpm_build_task_id}/build.log"
+        )
+
+    def get_koji_build_rpm_tasks_logs_urls(
+        self,
+        koji_logs_url: str = "https://kojipkgs.fedoraproject.org",
+    ) -> Dict[str, str]:
+        """
+        Constructs the log URLs for all RPM subtasks of the Koji task.
+        """
+        return {
+            arch: AbstractKojiEvent.get_koji_build_logs_url(
+                rpm_build_task_id=rpm_build_task_id,
+                koji_logs_url=koji_logs_url,
+            )
+            for arch, rpm_build_task_id in self.rpm_build_task_ids.items()
+        }
+
     def get_dict(self, default_dict: Optional[Dict] = None) -> dict:
         result = super().get_dict()
         result.pop("_build_model")
@@ -94,20 +117,21 @@ def __init__(
         epoch: str,
         version: str,
         release: str,
-        rpm_build_task_id: int,
+        task_id: int,
         web_url: Optional[str] = None,
         old_state: Optional[KojiBuildState] = None,
+        rpm_build_task_ids: Optional[Dict[str, int]] = None,
     ):
-        super().__init__(build_id)
+        super().__init__(task_id=task_id, rpm_build_task_ids=rpm_build_task_ids)
+        self.build_id = build_id
         self.state = state
         self.old_state = old_state
         self.package_name = package_name
-        self.rpm_build_task_id = rpm_build_task_id
+        self.task_id = task_id
         self.epoch = epoch
         self.version = version
         self.release = release
         self.web_url = web_url
-
         self.branch_name = branch_name
         self._commit_sha = commit_sha  # we know  it, no need to get it from db
         self.namespace = namespace
@@ -123,31 +147,6 @@ def commit_sha(self) -> Optional[str]:  # type:ignore
     def nvr(self) -> str:
         return f"{self.package_name}-{self.version}-{self.release}"
 
-    @property
-    def build_model(self) -> Optional[KojiBuildTargetModel]:
-        if not super().build_model:
-            _, event = ProjectEventModel.add_branch_push_event(
-                branch_name=self.branch_name,
-                repo_name=self.repo_name,
-                namespace=self.namespace,
-                project_url=self.project_url,
-                commit_sha=self._commit_sha,
-            )
-            group = KojiBuildGroupModel.create(
-                run_model=PipelineModel.create(
-                    project_event=event, package_name=self.package_name
-                )
-            )
-            self._build_model = KojiBuildTargetModel.create(
-                build_id=str(self.build_id),
-                web_url=self.web_url,
-                target="noarch",  # TODO: where to get this info from?
-                status=self.state.value,
-                scratch=True,  # used by the event for scratch builds
-                koji_build_group=group,
-            )
-        return self._build_model
-
     @property
     def git_ref(self) -> str:
         return self.branch_name
@@ -171,7 +170,8 @@ def from_event_dict(cls, event: dict):
             old_state=(
                 KojiBuildState(raw_old) if (raw_old := event.get("old_state")) else None
             ),
-            rpm_build_task_id=event.get("rpm_build_task_id"),
+            task_id=event.get("task_id"),
+            rpm_build_task_ids=event.get("rpm_build_task_ids"),
             package_name=event.get("package_name"),
             project_url=event.get("project_url"),
             web_url=event.get("web_url"),
@@ -192,14 +192,14 @@ class KojiTaskEvent(AbstractKojiEvent):
 
     def __init__(
         self,
-        build_id: int,
+        task_id: int,
         state: KojiTaskState,
         old_state: Optional[KojiTaskState] = None,
-        rpm_build_task_id: Optional[int] = None,
+        rpm_build_task_ids: Optional[Dict[str, int]] = None,
         start_time: Optional[Union[int, float, str]] = None,
         completion_time: Optional[Union[int, float, str]] = None,
     ):
-        super().__init__(build_id=build_id, rpm_build_task_id=rpm_build_task_id)
+        super().__init__(task_id=task_id, rpm_build_task_ids=rpm_build_task_ids)
         self.state = state
         self.old_state = old_state
         self.start_time: Optional[Union[int, float, str]] = start_time
@@ -256,14 +256,14 @@ def identifier(self) -> str:
     @classmethod
     def from_event_dict(cls, event: dict):
         return KojiTaskEvent(
-            build_id=event.get("build_id"),
+            task_id=event.get("build_id"),
             state=KojiTaskState(event.get("state")) if event.get("state") else None,
             old_state=(
                 KojiTaskState(event.get("old_state"))
                 if event.get("old_state")
                 else None
             ),
-            rpm_build_task_id=event.get("rpm_build_task_id"),
+            rpm_build_task_ids=event.get("rpm_build_task_ids"),
             start_time=event.get("start_time"),
             completion_time=event.get("completion_time"),
         )
@@ -291,18 +291,3 @@ def get_dict(self, default_dict: Optional[Dict] = None) -> dict:
         result["git_ref"] = self.git_ref
         result["identifier"] = self.identifier
         return result
-
-    @staticmethod
-    def get_koji_build_logs_url(
-        rpm_build_task_id: int,
-        koji_logs_url: str = "https://kojipkgs.fedoraproject.org",
-    ) -> str:
-        """
-        Constructs the log URL for the given Koji task.
-        You can redefine the Koji instance using the one defined in the service config.
-        TODO: this does not work for non-scratch builds
-        """
-        return (
-            f"{koji_logs_url}//work/tasks/"
-            f"{rpm_build_task_id % 10000}/{rpm_build_task_id}/build.log"
-        )