build logs: utilize our IDs instead of COPR build IDs

Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

packit_service/models.py

@@ -151,6 +151,11 @@ def set_build_logs_url(self, build_logs: str):
         session.add(self)
         session.commit()
 
+    @classmethod
+    def get_by_id(cls, id_: int) -> Optional["CoprBuild"]:
+        session = get_sa_session()
+        return session.query(CoprBuild).filter_by(id=id_).first()
+
     @classmethod
     def get_by_build_id(cls, build_id: str, target: str) -> Optional["CoprBuild"]:
         session = get_sa_session()

packit_service/service/events.py

@@ -27,13 +27,14 @@
 import enum
 import logging
 from datetime import datetime, timezone
-from typing import Optional, List, Union
+from typing import Optional, List, Union, Dict
 
 import requests
 from ogr.abstract import GitProject
 from packit.config import JobTriggerType, get_package_config_from_repo, PackageConfig
 
 from packit_service.config import ServiceConfig, GithubPackageConfigGetter
+from packit_service.models import CoprBuild
 from packit_service.worker.copr_db import CoprBuildDB
 from packit_service.worker.utils import get_copr_build_url_for_values
 
@@ -133,8 +134,9 @@ def ts2str(event: dict):
             event["created_at"] = datetime.fromtimestamp(created_at).isoformat()
         return event
 
-    def get_dict(self) -> dict:
-        d = copy.deepcopy(self.__dict__)
+    def get_dict(self, default_dict: Optional[Dict] = None) -> dict:
+        d = default_dict or self.__dict__
+        d = copy.deepcopy(d)
         # whole dict have to be JSON serializable because of redis
         d["trigger"] = d["trigger"].value
         d["created_at"] = int(d["created_at"].timestamp())
@@ -212,7 +214,7 @@ def __init__(
         self.commit_sha = commit_sha
         self.github_login = github_login
 
-    def get_dict(self) -> dict:
+    def get_dict(self, default_dict: Optional[Dict] = None) -> dict:
         result = super().get_dict()
         result["action"] = result["action"].value
         return result
@@ -255,7 +257,7 @@ def __init__(
         self.github_login = github_login
         self.comment = comment
 
-    def get_dict(self) -> dict:
+    def get_dict(self, default_dict: Optional[Dict] = None) -> dict:
         result = super().get_dict()
         result["action"] = result["action"].value
         return result
@@ -302,7 +304,7 @@ def __init__(
         self.github_login = github_login
         self.comment = comment
 
-    def get_dict(self) -> dict:
+    def get_dict(self, default_dict: Optional[Dict] = None) -> dict:
         result = super().get_dict()
         result["action"] = result["action"].value
         return result
@@ -349,7 +351,7 @@ def __init__(
         self.sender_login = sender_login
         self.status = status
 
-    def get_dict(self) -> dict:
+    def get_dict(self, default_dict: Optional[Dict] = None) -> dict:
         result = super().get_dict()
         result["status"] = result["status"].value
         return result
@@ -375,7 +377,7 @@ def __init__(
         self.msg_id = msg_id
         self.project_url = project_url
 
-    def get_dict(self) -> dict:
+    def get_dict(self, default_dict: Optional[Dict] = None) -> dict:
         result = super().get_dict()
         result["topic"] = result["topic"].value
         return result
@@ -420,7 +422,7 @@ def __init__(
         self.ref: str = ref
         self.commit_sha: str = commit_sha
 
-    def get_dict(self) -> dict:
+    def get_dict(self, default_dict: Optional[Dict] = None) -> dict:
         result = super().get_dict()
         result["result"] = result["result"].value
         return result
@@ -447,8 +449,25 @@ def __init__(
         owner: str,
         project_name: str,
         pkg: str,
+        build_pg: Optional[CoprBuild] = None,
     ):
-        super().__init__(trigger=JobTriggerType.commit, project_url=build["https_url"])
+        if build_pg:
+            self.pr_id = build_pg.pr.pr_id
+            self.ref = ""  # we don't have that in PG, yet
+            self.commit_sha = build_pg.commit_sha
+            self.base_repo_name = build_pg.pr.project.repo_name
+            self.base_repo_namespace = build_pg.pr.project.namespace
+            # FIXME: hardcoded, move this to PG
+            https_url = f"https://github.com/{self.base_repo_namespace}/{self.base_repo_name}.git"
+        else:
+            self.pr_id = build.get("pr_id")
+            self.ref = build.get("ref", "")
+            self.commit_sha = build.get("commit_sha", "")
+            self.base_repo_name = build.get("repo_name")
+            self.base_repo_namespace = build.get("repo_namespace")
+            https_url = build["https_url"]
+
+        super().__init__(trigger=JobTriggerType.commit, project_url=https_url)
         self.topic = FedmsgTopic(topic)
         self.build_id = build_id
         self.build = build
@@ -457,16 +476,7 @@ def __init__(
         self.owner = owner
         self.project_name = project_name
         self.pkg = pkg
-        self.base_repo_name = ""
-        self.base_repo_namespace = ""
-        self.pr_id = 0
-        self.ref = ""
-        self.commit_sha = ""
-        self.base_repo_name = build.get("repo_name")
-        self.base_repo_namespace = build.get("repo_namespace")
-        self.pr_id = build.get("pr_id")
-        self.ref = build.get("ref")
-        self.commit_sha = build.get("commit_sha")
+        self.build_pg = build_pg
 
     @classmethod
     def from_build_id(
@@ -480,21 +490,38 @@ def from_build_id(
         pkg: str,
     ) -> Optional["CoprBuildEvent"]:
         """ Return cls instance or None if build_id not in CoprBuildDB"""
-        build = CoprBuildDB().get_build(build_id)
-        if not build:
-            logger.warning(f"Build id: {build_id} not in CoprBuildDB")
-            return None
-        return cls(topic, build_id, build, chroot, status, owner, project_name, pkg)
+        # pg
+        build_pg = CoprBuild.get_by_build_id(str(build_id), chroot)
+        build = None
+        if not build_pg:
+            # let's try redis now
+            build = CoprBuildDB().get_build(build_id)
+            if not build:
+                logger.warning(f"Build id: {build_id} not in CoprBuildDB")
+                return None
+        return cls(
+            topic,
+            build_id,
+            build,
+            chroot,
+            status,
+            owner,
+            project_name,
+            pkg,
+            build_pg=build_pg,
+        )
 
     def pre_check(self):
-        if not self.build:
+        if not self.build and not self.build_pg:
             logger.warning("Copr build is not handled by this deployment.")
             return False
 
         return True
 
-    def get_dict(self) -> dict:
-        result = super().get_dict()
+    def get_dict(self, default_dict: Optional[Dict] = None) -> dict:
+        d = self.__dict__
+        d.pop("build_pg")
+        result = super().get_dict(d)
         result["topic"] = result["topic"].value
         return result
 

packit_service/service/urls.py

@@ -19,35 +19,20 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
-from typing import Optional
 
 from flask import url_for
 
 from packit_service.service.app import application
-from packit_service.service.events import CoprBuildEvent
 
 
-def get_log_url(
-    event: Optional[CoprBuildEvent] = None,
-    target: Optional[str] = None,
-    build_id: Optional[int] = None,
-) -> str:
+def get_log_url(id_: int = None,) -> str:
     """
     provide absolute URL to p-s build logs view meant to set in a commit status
     """
     # flask magic
     with application.app_context():
-        if event:
-            return url_for(
-                "builds.get_build_logs",
-                build_id=event.build_id,
-                target=event.chroot,
-                _external=True,  # _external = generate a URL with FQDN, not a relative one
-            )
-        else:
-            return url_for(
-                "builds.get_build_logs",
-                build_id=build_id,
-                target=target,
-                _external=True,
-            )
+        return url_for(
+            "builds.get_build_logs_by_id",
+            id_=id_,
+            _external=True,  # _external = generate a URL with FQDN, not a relative one
+        )

packit_service/service/views.py

@@ -30,17 +30,16 @@
 builds_blueprint = Blueprint("builds", __name__)
 
 
-@builds_blueprint.route(
-    "/build/<string:build_id>/<string:target>/logs", methods=("GET",)
-)
-def get_build_logs(build_id, target):
-    build = CoprBuild.get_by_build_id(build_id, target)
+@builds_blueprint.route("/copr-build/<int:id_>/logs", methods=("GET",))
+def get_build_logs_by_id(id_):
+    build = CoprBuild.get_by_id(id_)
     if build:
         response = (
             "<html><head>"
             f"<title>Build {build.pr.project.namespace}/{build.pr.project.repo_name}"
             f" #{build.pr.pr_id}</title></head><body>"
-            f"Build {build_id} is in state {build.status}<br><br>"
+            f"COPR Build ID: {build.build_id}<br>"
+            f"State: {build.status}<br><br>"
             f'Build web interface URL: <a href="{build.web_url}">{build.web_url}</a><br>'
         )
         if build.build_logs_url:
@@ -54,4 +53,4 @@ def get_build_logs(build_id, target):
             "<br></body></html>"
         )
         return response
-    return f"We can't find any info about build {build_id}.\n"
+    return f"We can't find any info about build {id_}.\n"

packit_service/worker/copr_build.py

@@ -307,11 +307,9 @@ def report_status_to_build_for_chroot(
         self, description, state, url: str = "", chroot: str = ""
     ):
         if self.job_copr_build and chroot in self.build_chroots:
+            cs = PRCheckName.get_build_check(chroot)
             self.status_reporter.report(
-                description=description,
-                state=state,
-                url=url,
-                check_names=PRCheckName.get_build_check(chroot),
+                description=description, state=state, url=url, check_names=cs,
             )
 
     def report_status_to_test_for_chroot(
@@ -363,14 +361,7 @@ def run_copr_build(self) -> HandlerResults:
             status = "pending"
             description = "Building RPM ..."
             for chroot in self.build_chroots:
-                url = get_log_url(build_id=build_id, target=chroot)
-                self.status_reporter.report(
-                    state=status,
-                    description=description,
-                    url=url,
-                    check_names=PRCheckName.get_build_check(chroot),
-                )
-                CoprBuild.get_or_create(
+                copr_build = CoprBuild.get_or_create(
                     pr_id=self.event.pr_id,
                     build_id=str(build_id),
                     commit_sha=self.event.commit_sha,
@@ -381,14 +372,20 @@ def run_copr_build(self) -> HandlerResults:
                     status=status,
                     srpm_build=srpm_build,
                 )
-            for chroot in self.tests_chroots:
-                url = get_log_url(build_id=build_id, target=chroot)
+                url = get_log_url(id_=copr_build.id)
                 self.status_reporter.report(
                     state=status,
                     description=description,
                     url=url,
-                    check_names=PRCheckName.get_testing_farm_check(chroot),
+                    check_names=PRCheckName.get_build_check(chroot),
                 )
+                if chroot in self.tests_chroots:
+                    self.status_reporter.report(
+                        state=status,
+                        description=description,
+                        url=url,
+                        check_names=PRCheckName.get_testing_farm_check(chroot),
+                    )
 
         except SandcastleTimeoutReached:
             return self._process_timeout()

packit_service/worker/fedmsg_handlers.py

@@ -212,6 +212,7 @@ def run(self):
             msg = "SRPM build in copr has finished"
             logger.debug(msg)
             return HandlerResults(success=True, details={"msg": msg})
+        # TODO: drop the code below once we move to PG completely; the build is present in event
         # pg
         build_pg = CoprBuild.get_by_build_id(
             str(self.event.build_id), self.event.chroot
@@ -230,7 +231,7 @@ def run(self):
                 return HandlerResults(success=False, details={"msg": msg})
 
         if build_pg:
-            url = get_log_url(self.event)
+            url = get_log_url(build_pg.id)
         else:
             url = copr_url_from_event(self.event)
         gh_state = "failure"
@@ -310,6 +311,7 @@ def run(self):
             logger.debug(msg)
             return HandlerResults(success=True, details={"msg": msg})
 
+        # TODO: drop the code below once we move to PG completely; the build is present in event
         # pg
         build_pg = CoprBuild.get_by_build_id(self.event.build_id, self.event.chroot)
         if not build_pg:
@@ -325,11 +327,12 @@ def run(self):
                 logger.warning(msg)
                 return HandlerResults(success=False, details={"msg": msg})
 
-        url = get_log_url(self.event)
-
         status = "pending"
         if build_pg:
+            url = get_log_url(build_pg.id)
             build_pg.set_status(status)
+        else:
+            url = copr_url_from_event(self.event)
 
         self.build_job_helper.report_status_for_chroot(
             description="RPM build has started...",

tests/conftest.py

@@ -32,6 +32,7 @@
 
 from packit_service.config import ServiceConfig
 from packit_service.constants import SANDCASTLE_WORK_DIR
+from packit_service.models import GitProject, PullRequest, CoprBuild
 from packit_service.worker.whitelist import Whitelist
 from tests.spellbook import SAVED_HTTPD_REQS
 
@@ -149,3 +150,13 @@ def mock_issue_comment_functionality():
     flexmock(ServiceConfig).should_receive("get_service_config").and_return(config)
     flexmock(LocalProject, refresh_the_arguments=lambda: None)
     flexmock(Whitelist, check_and_report=True)
+
+
+@pytest.fixture()
+def copr_build():
+    proj = GitProject(id=1, repo_name="bar", namespace="foo")
+    pr = PullRequest(id=1, pr_id=123)
+    pr.project = proj
+    c = CoprBuild(id=1)
+    c.pr = pr
+    return c