#1780 Store build results to the database
Merged 3 years ago by praiskup. Opened 3 years ago by frostyx.
copr/ frostyx/copr api-build-results  into  main

@@ -9,6 +9,7 @@ 

  import shutil

  import statistics

  import time

+ import json

  

  from packaging import version

  
@@ -30,7 +31,7 @@ 

  

  MAX_HOST_ATTEMPTS = 3

  MAX_SSH_ATTEMPTS = 5

- MIN_BUILDER_VERSION = "0.40.1.dev"

+ MIN_BUILDER_VERSION = "0.49.1.dev"

  CANCEL_CHECK_PERIOD = 5

  

  MESSAGES = {
@@ -281,6 +282,20 @@ 

          self.frontend_client.update(data)

          self.sender.announce("build.end", self.job, self.last_hostname)

  

+     def _parse_results(self):

+         """

+         Parse `results.json` and update the `self.job` object.

+         """

+         if self.job.chroot == "srpm-builds":

+             # We care only about final RPMs

+             return

+ 

+         path = os.path.join(self.job.results_dir, "results.json")

+         assert os.path.exists(path)

+         with open(path, "r") as f:

+             results = json.load(f)

+         self.job.results = results

+ 

      def _wait_for_repo(self):

          """

          Wait a while for initial createrepo, and eventually fail the build
@@ -650,6 +665,7 @@ 

                  build_details = {

                      "built_packages": self._collect_built_packages(job),

                  }

+                 self._parse_results()

              self.log.info("build details: %s", build_details)

          except Exception as e:

              raise BackendError(

@@ -66,6 +66,8 @@ 

          self.uses_devel_repo = None

          self.sandbox = None

  

+         self.results = None

+ 

          # TODO: validate update data, user marshmallow

          for key, val in task_data.items():

              key = str(key)

@@ -0,0 +1,11 @@ 

+ {

+    "packages": [

+       {

+          "name":"example",

+          "epoch":0,

+          "version":"1.0.14",

+          "release":"1.fc30",

+          "arch":"x86_64"

+       }

+    ]

+ }

@@ -268,7 +268,8 @@ 

      assert (logging.INFO, MESSAGES["repo_waiting"]) \

          in [(r[1], r[2]) for r in caplog.record_tuples]

  

- def test_full_rpm_build_no_sign(f_build_rpm_case, caplog):

+ @_patch_bwbuild_object("BuildBackgroundWorker._parse_results")

+ def test_full_rpm_build_no_sign(_parse_results, f_build_rpm_case, caplog):

      """

      Go through the whole (successful) build of a binary RPM

      """
@@ -321,7 +322,8 @@ 

  

  @mock.patch("copr_backend.sign.SIGN_BINARY", "tests/fake-bin-sign")

  @mock.patch("copr_backend.sign._sign_one")

- def test_build_and_sign(mc_sign_one, f_build_rpm_sign_on, caplog):

+ @_patch_bwbuild_object("BuildBackgroundWorker._parse_results")

+ def test_build_and_sign(_parse_results, mc_sign_one, f_build_rpm_sign_on, caplog):

      config = f_build_rpm_sign_on

      worker = config.bw

      worker.process()
@@ -570,8 +572,9 @@ 

      ], caplog)

  

  @_patch_bwbuild_object("CANCEL_CHECK_PERIOD", 0.5)

+ @_patch_bwbuild_object("BuildBackgroundWorker._parse_results")

  @mock.patch("copr_backend.sign.SIGN_BINARY", "tests/fake-bin-sign")

- def test_build_retry(f_build_rpm_sign_on):

+ def test_build_retry(_parse_results, f_build_rpm_sign_on):

      config = f_build_rpm_sign_on

      worker = config.bw

      class _SideEffect():
@@ -680,7 +683,8 @@ 

          COMMON_MSGS["not finished"],

      ], caplog)

  

- def test_ssh_connection_error(f_build_rpm_case, caplog):

+ @_patch_bwbuild_object("BuildBackgroundWorker._parse_results")

+ def test_ssh_connection_error(_parse_results, f_build_rpm_case, caplog):

      class _SideEffect:

          counter = 0

          def __call__(self):
@@ -707,7 +711,9 @@ 

  

  @_patch_bwbuild_object("time.sleep", mock.MagicMock())

  @_patch_bwbuild_object("time.time")

- def test_retry_for_ssh_tail_failure(mc_time, f_build_rpm_case, caplog):

+ @_patch_bwbuild_object("BuildBackgroundWorker._parse_results")

+ def test_retry_for_ssh_tail_failure(_parse_results, mc_time, f_build_rpm_case,

+                                     caplog):

      mc_time.side_effect = list(range(500))

      class _SideEffect:

          counter = 0
@@ -766,7 +772,8 @@ 

      ], caplog)

      assert worker.job.status == 0  # fail

  

- def test_existing_compressed_file(f_build_rpm_case, caplog):

+ @_patch_bwbuild_object("BuildBackgroundWorker._parse_results")

+ def test_existing_compressed_file(_parse_results, f_build_rpm_case, caplog):

      config = f_build_rpm_case

      config.ssh.precreate_compressed_log_file = True

      worker = config.bw
@@ -777,7 +784,8 @@ 

          "Finished build: id=848963 failed=False ",  # still success!

      ], caplog)

  

- def test_tail_f_nonzero_exit(f_build_rpm_case, caplog):

+ @_patch_bwbuild_object("BuildBackgroundWorker._parse_results")

+ def test_tail_f_nonzero_exit(_parse_results, f_build_rpm_case, caplog):

      config = f_build_rpm_case

      worker = config.bw

      class _SideEffect:

@@ -0,0 +1,93 @@ 

+ #!/bin/bash

+ 

+ # Include Beaker environment

+ . /usr/bin/rhts-environment.sh || exit 1

+ . /usr/share/beakerlib/beakerlib.sh || exit 1

+ 

+ # Load config settings

+ HERE=$(dirname "$(realpath "$0")")

+ source "$HERE/config"

+ source "$HERE/helpers"

+ 

+ 

+ rlJournalStart

+     rlPhaseStartSetup

+         setup_checks

+         RESULTDIR=`mktemp -d`

+     rlPhaseEnd

+ 

+     rlPhaseStartTest

+         rlRun "copr-cli create ${NAME_PREFIX}TestResultsJson --chroot $CHROOT" 0

+         rlRun -s "copr-cli build ${NAME_PREFIX}TestResultsJson $HELLO --nowait" 0

+         rlRun "parse_build_id"

+         rlRun "copr watch-build $BUILD_ID"

+ 

+ 	checkResults()

+ 	{

+             rlAssertEquals "There should be 4 results" `cat $RESULTDIR/results.json |jq '.packages | length'` 4

+ 	    path=$1

+ 	    rlRun "cat $path |jq -e '.packages[0].name == \"hello-debugsource\"'"

+ 	    rlRun "cat $path |jq -e '.packages[0].epoch == 0'"

+ 	    rlRun "cat $path |jq -e '.packages[0].version == \"2.8\"'"

+ 	    rlRun "cat $path |jq -e '.packages[0].release == \"1.fc$FEDORA_VERSION\"'"

+ 	    rlRun "cat $path |jq -e '.packages[0].arch == \"x86_64\"'"

+ 

+ 	    rlRun "cat $path |jq -e '.packages[1].name == \"hello\"'"

+ 	    rlRun "cat $path |jq -e '.packages[1].epoch == 0'"

+ 	    rlRun "cat $path |jq -e '.packages[1].version == \"2.8\"'"

+ 	    rlRun "cat $path |jq -e '.packages[1].release == \"1.fc$FEDORA_VERSION\"'"

+ 	    rlRun "cat $path |jq -e '.packages[1].arch == \"x86_64\"'"

+ 

+ 	    rlRun "cat $path |jq -e '.packages[2].name == \"hello\"'"

+ 	    rlRun "cat $path |jq -e '.packages[2].epoch == 0'"

+ 	    rlRun "cat $path |jq -e '.packages[2].version == \"2.8\"'"

+ 	    rlRun "cat $path |jq -e '.packages[2].release == \"1.fc$FEDORA_VERSION\"'"

+ 	    rlRun "cat $path |jq -e '.packages[2].arch == \"src\"'"

+ 

+ 	    rlRun "cat $path |jq -e '.packages[3].name == \"hello-debuginfo\"'"

+ 	    rlRun "cat $path |jq -e '.packages[3].epoch == 0'"

+ 	    rlRun "cat $path |jq -e '.packages[3].version == \"2.8\"'"

+ 	    rlRun "cat $path |jq -e '.packages[3].release == \"1.fc$FEDORA_VERSION\"'"

+ 	    rlRun "cat $path |jq -e '.packages[3].arch == \"x86_64\"'"

+ 	}

+ 

+ 	# Check the results.json file that is stored on backend

+         URL_PATH="results/${NAME_PREFIX}TestResultsJson/$CHROOT/$(build_id_with_leading_zeroes)-hello/results.json"

+         rlRun "wget -P $RESULTDIR $BACKEND_URL/$URL_PATH"

+ 	checkResults "$RESULTDIR/results.json"

+ 

+ 

+ 	# Check the /build-chroot/built-packages/ APIv3 route

+         python << END

+ from copr.v3 import Client

+ from copr_cli.util import json_dumps

Aha, we don't have command line api for this functionality ATM. Sorry for pushing you this way .... pretty complicated test, but still good!

+ 

+ client = Client.create_from_config_file()

+ response = client.build_chroot_proxy.get_built_packages($BUILD_ID, "$CHROOT")

+ with open("$RESULTDIR/results-api-build-chroot.json", "w") as f:

+     f.write(json_dumps(response))

+ END

+ 	checkResults "$RESULTDIR/results-api-build-chroot.json"

+ 

+ 

+ 	# Check the /build/built-packages/ APIv3 route

+         python << END

+ from copr.v3 import Client

+ from copr_cli.util import json_dumps

+ 

+ client = Client.create_from_config_file()

+ response = client.build_proxy.get_built_packages($BUILD_ID)

+ with open("$RESULTDIR/results-api-build.json", "w") as f:

+     f.write(json_dumps(response["$CHROOT"]))

+ END

+ 	checkResults "$RESULTDIR/results-api-build.json"

+ 

+ 

+     rlPhaseEnd

+ 

+     rlPhaseStartCleanup

+         cleanProject "${NAME_PREFIX}TestResultsJson"

+         rlRun "rm -rf $RESULTDIR/*"

+     rlPhaseEnd

+ rlJournalPrintText

+ rlJournalEnd

@@ -0,0 +1,33 @@ 

+ """

+ Add BuildChrootResults

+ 

+ Revision ID: efec6b1aa9a2

+ Revises: d8a1062ee4cf

+ Create Date: 2021-05-13 16:48:05.569521

+ """

+ 

+ import sqlalchemy as sa

+ from alembic import op

+ 

+ 

+ revision = 'efec6b1aa9a2'

+ down_revision = 'd8a1062ee4cf'

+ 

+ def upgrade():

+     op.create_table('build_chroot_result',

+ 

+     sa.Column('id', sa.Integer(), nullable=False),

+     sa.Column('build_chroot_id', sa.Integer(), nullable=False),

+     sa.Column('name', sa.Text(), nullable=False),

+     sa.Column('epoch', sa.Integer(), default=0),

+     sa.Column('version', sa.Text(), nullable=False),

+     sa.Column('release', sa.Text(), nullable=False),

+     sa.Column('arch', sa.Text(), nullable=False),

+ 

+     sa.ForeignKeyConstraint(['build_chroot_id'], ['build_chroot.id'],

+                             ondelete='CASCADE'),

+     sa.PrimaryKeyConstraint('id')

+     )

+ 

+ def downgrade():

+     op.drop_table('build_chroot_result')

@@ -965,6 +965,10 @@ 

  

                      if upd_dict.get("status") in BuildsLogic.terminal_states:

                          build_chroot.ended_on = upd_dict.get("ended_on") or time.time()

+                         assert isinstance(upd_dict, dict)

+                         assert not isinstance(upd_dict, str)

+                         BuildChrootResultsLogic.create_from_dict(

+                             build_chroot, upd_dict.get("results"))

  

                      if upd_dict.get("status") == StatusEnum("starting"):

                          build_chroot.started_on = upd_dict.get("started_on") or time.time()
@@ -1302,6 +1306,24 @@ 

          return query

  

      @classmethod

+     def get_by_results(cls, **kwargs):

+         """

+         Query all `BuildChroot` instances whose `results` corresponds with the

+         specified `kwargs`.

+ 

+         Supported parameter names:

+ 

+             name, epoch, version, release, arch

+ 

+         Example usage:

+ 

+             cls.get_by_results(name="hello")

+             cls.get_by_results(name="foo", arch="x86_64")

+         """

+         return (models.BuildChroot.query

+                 .filter(models.BuildChroot.results.any(**kwargs)))

+ 

+     @classmethod

      def filter_by_build_id(cls, query, build_id):

          return query.filter(models.Build.id == build_id)

  
@@ -1342,6 +1364,39 @@ 

          return cls.filter_by_copr_and_mock_chroot(BuildChroot.query, copr,

                                                    mock_chroot)

  

+ class BuildChrootResultsLogic:

+     """

+     High-level interface for working with `models.BuildChrootResult` objects

+     """

+ 

+     @classmethod

+     def create(cls, build_chroot, name, epoch, version, release, arch):

+         """

+         Create a new record about a built package in some `BuildChroot`

+         """

+         return models.BuildChrootResult(

+             build_chroot_id=build_chroot,

+             build_chroot=build_chroot,

+             name=name,

+             epoch=epoch,

+             version=version,

+             release=release,

+             arch=arch,

+         )

+ 

+ 

+     @classmethod

+     def create_from_dict(cls, build_chroot, results):

+         """

+         Parses a `dict` in the following format

+ 

+             {"packages": [{"name": "foo", "epoch": 0, ...}]}

+ 

+         and records all of the built packages for a given `BuildChroot`.

+         """

+         return [cls.create(build_chroot, **result)

+                 for result in results["packages"]]

+ 

  

  class BuildsMonitorLogic(object):

      @classmethod

@@ -1464,6 +1464,13 @@ 

          """

          return [ch for ch in self.chroots if ch in self.copr.active_chroots]

  

+     @property

+     def results_dict(self):

+         """

+         Built packages in each build chroot.

+         """

+         return {bc.name: bc.results_dict for bc in self.build_chroots}

+ 

  

  class DistGitBranch(db.Model, helpers.Serializer):

      """
@@ -1949,6 +1956,43 @@ 

              logs.append(log)

          return logs

  

+     @property

+     def results_dict(self):

+         """

+         Returns a `dict` containing all built packages in this chroot

+         """

+         built_packages = []

+         for result in self.results:

+             options = {"__columns_except__": ["id", "build_chroot_id"]}

+             result_dict= result.to_dict(options=options)

+             built_packages.append(result_dict)

+         return {"packages": built_packages}

+ 

+ 

+ 

+ class BuildChrootResult(db.Model, helpers.Serializer):

+     """

+     Represents a built package within some `BuildChroot`

+     """

+ 

+     id = db.Column(db.Integer, primary_key=True)

+     build_chroot_id = db.Column(

+         db.Integer,

+         db.ForeignKey("build_chroot.id"),

+         nullable=False

+     )

+ 

+     name = db.Column(db.Text, nullable=False)

+     epoch = db.Column(db.Integer, default=0)

+     version = db.Column(db.Text, nullable=False)

+     release = db.Column(db.Text, nullable=False)

+     arch = db.Column(db.Text, nullable=False)

+ 

+     build_chroot = db.relationship(

+         "BuildChroot",

+         backref=db.backref("results", cascade="all, delete-orphan"),

+     )

+ 

  

  class LegalFlag(db.Model, helpers.Serializer):

      id = db.Column(db.Integer, primary_key=True)

@@ -64,3 +64,13 @@ 

  def get_build_chroot_config(build_id, chrootname):

      chroot = ComplexLogic.get_build_chroot(build_id, chrootname)

      return flask.jsonify(build_config(chroot))

+ 

+ 

+ @apiv3_ns.route("/build-chroot/built-packages/", methods=GET)

+ @query_params()

+ def get_build_chroot_built_packages(build_id, chrootname):

+     """

+     Return built packages (NEVRA dicts) for a given build chroot

+     """

+     chroot = ComplexLogic.get_build_chroot(build_id, chrootname)

+     return flask.jsonify(chroot.results_dict)

@@ -115,6 +115,15 @@ 

      return flask.jsonify(to_source_build_config(build))

  

  

+ @apiv3_ns.route("/build/built-packages/<int:build_id>/", methods=GET)

+ def get_build_built_packages(build_id):

+     """

+     Return built packages (NEVRA dicts) for a given build

+     """

+     build = ComplexLogic.get_build_safe(build_id)

+     return flask.jsonify(build.results_dict)

+ 

+ 

  @apiv3_ns.route("/build/cancel/<int:build_id>", methods=PUT)

  @api_login_required

  def cancel_build(build_id):

@@ -0,0 +1,45 @@ 

+ """

+ Test all kind of build chroot request via APIv3

+ """

+ 

+ import pytest

+ from tests.coprs_test_case import CoprsTestCase, TransactionDecorator

+ from coprs.logic.builds_logic import BuildChrootResultsLogic

+ 

+ 

+ class TestAPIv3BuildChrootsResults(CoprsTestCase):

+     """

+     Tests related to build chroots results

+     """

+ 

+     @TransactionDecorator("u1")

+     @pytest.mark.usefixtures("f_users", "f_users_api", "f_coprs",

+                              "f_mock_chroots", "f_builds", "f_db")

+     def test_build_chroot_built_packages(self):

+         """

+         Test the endpoint for getting built packages (NEVRA dicts) for a given

+         build chroot.

+         """

+         self.db.session.add(self.b1, self.b1_bc)

+         built_packages = {

+             "packages": [

+                 {

+                     "name": "hello",

+                     "epoch": 0,

+                     "version": "2.8",

+                     "release": "1.fc33",

+                     "arch": "x86_64"

+                 },

+             ]

+         }

+         BuildChrootResultsLogic.create_from_dict(

+             self.b1.build_chroots[0], built_packages)

+         self.db.session.commit()

+ 

+         endpoint = "/api_3/build-chroot/built-packages/"

+         endpoint += "?build_id={build_id}&chrootname={chrootname}"

+         params = {"build_id": self.b1.id, "chrootname": "fedora-18-x86_64"}

+ 

+         result = self.tc.get(endpoint.format(**params))

+         assert result.is_json

+         assert result.json == built_packages

@@ -9,6 +9,7 @@ 

  

  from bs4 import BeautifulSoup

  from copr_common.enums import BuildSourceEnum

+ from coprs.logic.builds_logic import BuildChrootResultsLogic

  

  from tests.coprs_test_case import CoprsTestCase, TransactionDecorator

  
@@ -238,3 +239,35 @@ 

  

          resp = self.test_client.get(route)

          assert get_selected(resp.data)["value"] == "simple"

+ 

+ 

+ class TestAPIv3BuildsResults(CoprsTestCase):

+     """

+     Tests related to build results

+     """

+ 

+     @TransactionDecorator("u1")

+     @pytest.mark.usefixtures("f_users", "f_users_api", "f_coprs",

+                              "f_mock_chroots", "f_builds", "f_db")

+     def test_build_built_packages(self):

+         """

+         Test the endpoint for getting built packages (NEVRA dicts) for a given

+         build.

+         """

+         self.db.session.add(self.b1, self.b1_bc)

+         nevra = {

+             "name": "hello",

+             "epoch": 0,

+             "version": "2.8",

+             "release": "1.fc33",

+             "arch": "x86_64"

+         }

+         built_packages = {"packages": [nevra]}

+         BuildChrootResultsLogic.create_from_dict(

+             self.b1.build_chroots[0], built_packages)

+         self.db.session.commit()

+ 

+         endpoint = "/api_3/build/built-packages/{0}/".format(self.b1.id)

+         result = self.tc.get(endpoint)

+         assert result.is_json

+         assert result.json["fedora-18-x86_64"] == built_packages

@@ -16,7 +16,11 @@ 

                                InsufficientStorage)

  

  from coprs.logic.actions_logic import ActionsLogic

- from coprs.logic.builds_logic import BuildsLogic

+ from coprs.logic.builds_logic import (

+     BuildsLogic,

+     BuildChrootsLogic,

+     BuildChrootResultsLogic,

+ )

  

  from tests.coprs_test_case import CoprsTestCase, TransactionDecorator

  
@@ -510,3 +514,48 @@ 

          assert len(build.build_chroots) == 1

          assert build.source_status == StatusEnum("importing")

          assert build.package.name == "foo"

+ 

+     @pytest.mark.usefixtures("f_users", "f_coprs", "f_mock_chroots", "f_builds", "f_db")

+     def test_build_results_filter(self):

+         """

+         Test that we can query `BuildChroot` instances based on their `results`.

+         """

+         defaults = {

+             "name": None,

+             "epoch": 0,

+             "version": "1.0",

+             "release": "1",

+             "arch": None,

+         }

+ 

+         b1b2_chroots = self.b1.build_chroots + self.b2.build_chroots

+         for chroot in b1b2_chroots:

+             result  = defaults | {"name": "foo", "arch": "x86_64"}

+             results = {"packages": [result]}

+             BuildChrootResultsLogic.create_from_dict(chroot, results)

+ 

+         for chroot in self.b3.build_chroots:

+             result = defaults | {"name": "bar", "arch": "noarch"}

+             results = {"packages": [result]}

+             BuildChrootResultsLogic.create_from_dict(chroot, results)

+ 

+         for chroot in self.b4.build_chroots:

+             result1 = defaults | {"name": "foobar", "arch": "ppc64le"}

+             result2 = defaults | {"name": "qux", "arch": "ppc64le"}

+             results = {"packages": [result1, result2]}

+             BuildChrootResultsLogic.create_from_dict(chroot, results)

+ 

+         # Filter results by name

+         result = BuildChrootsLogic.get_by_results(name="foo").all()

+         assert set(result) == set(b1b2_chroots)

+ 

+         # Filter results by multiple attributes

+         result = BuildChrootsLogic.get_by_results(name="foo", arch="noarch").all()

+         assert result == []

+ 

+         result = BuildChrootsLogic.get_by_results(name="foo", arch="x86_64").all()

+         assert set(result) == set(b1b2_chroots)

+ 

+         # Filter results with multiple rows

+         result = BuildChrootsLogic.get_by_results(name="qux").all()

+         assert set(result) == set(self.b4.build_chroots)

@@ -147,6 +147,19 @@ 

  # status = 0 # failure

  # status = 1 # succeeded

  class TestUpdateBuilds(CoprsTestCase):

+     built_packages = """

+ {

+   "packages":[

+     {

+       "name":"example",

+       "epoch":0,

+       "version":"1.0.14",

+       "release":"1.fc30",

+       "arch":"x86_64"

+     }

+   ]

+ }"""

+ 

      data1 = """

  {

    "builds":[
@@ -168,6 +181,17 @@ 

       "status": 1,

       "chroot": "fedora-18-x86_64",

       "result_dir": "bar",

+      "results": {

+        "packages":[

+          {

+            "name":"example",

+            "epoch":0,

+            "version":"1.0.14",

+            "release":"1.fc30",

+            "arch":"x86_64"

+          }

+        ]

+      },

       "ended_on": 1490866440

     }

    ]
@@ -190,6 +214,7 @@ 

       "status": 0,

       "chroot": "fedora-18-x86_64",

       "result_dir": "bar",

+      "results": {"packages": []},

       "ended_on": 1390866440

     },

     {
@@ -198,6 +223,7 @@ 

       "status": 0,

       "chroot": "fedora-18-x86_64",

       "result_dir": "bar",

+      "results": {"packages": []},

       "ended_on": 1390866440

     },

     {

@@ -45,6 +45,18 @@ 

          response = request.send()

          return munchify(response)

  

+     def get_built_packages(self, build_id):

+         """

+         Return built packages (NEVRA dicts) for a given build

+ 

+         :param int build_id:

+         :return: Munch

+         """

+         endpoint = "/build/built-packages/{0}".format(build_id)

+         request = Request(endpoint, api_base_url=self.api_base_url)

+         response = request.send()

+         return munchify(response)

+ 

      def get_list(self, ownername, projectname, packagename=None, status=None, pagination=None):

          """

          Return a list of packages

@@ -58,3 +58,20 @@ 

          request = Request(endpoint, api_base_url=self.api_base_url, params=params)

          response = request.send()

          return munchify(response)

+ 

+     def get_built_packages(self, build_id, chrootname):

+         """

+         Return built packages (NEVRA dicts) for a given build chroot

+ 

+         :param int build_id:

+         :param str chrootname:

+         :return: Munch

+         """

+         endpoint = "/build-chroot/built-packages"

+         params = {

+             "build_id": build_id,

+             "chrootname": chrootname,

+         }

+         request = Request(endpoint, api_base_url=self.api_base_url, params=params)

+         response = request.send()

+         return munchify(response)

@@ -45,6 +45,7 @@ 

  BuildRequires: %{python_pfx}-munch

  BuildRequires: %{python}-requests

  BuildRequires: %{python_pfx}-jinja2

+ BuildRequires: %{python_pfx}-simplejson

  

  %if 0%{?fedora} || 0%{?rhel} > 7

  BuildRequires: argparse-manpage

@@ -4,6 +4,7 @@ 

  """

  

  from copr_rpmbuild.automation.fedora_review import FedoraReview

+ from copr_rpmbuild.automation.rpm_results import RPMResults

  

  

  def run_automation_tools(task, resultdir, mock_config_file, log):
@@ -11,7 +12,7 @@ 

      Iterate over all supported post-build tools (e.g. `fedora-review`,

      `rpmlint`, etc) and run the desired ones for a given task.

      """

-     tools = [FedoraReview]

+     tools = [FedoraReview, RPMResults]

nice :-)

      for _class in tools:

          tool = _class(task, resultdir, mock_config_file, log)

          if not tool.enabled:

@@ -0,0 +1,75 @@ 

+ """

+ Create `results.json` file

+ """

+ 

+ import os

+ import rpm

+ import simplejson

+ from copr_rpmbuild.automation.base import AutomationTool

+ 

+ 

+ class RPMResults(AutomationTool):

+     """

+     Create `results.json` file containing NEVRAs for all built RPM files

+     """

+ 

+     @property

+     def enabled(self):

+         """

+         Do this for every build

+         """

+         return True

+ 

+     def run(self):

+         """

+         Create `results.json`

+         """

+         nevras = self.find_results_nevras_dicts()

+         packages = {"packages": nevras}

+         path = os.path.join(self.resultdir, "results.json")

+         with open(path, "w") as dst:

+             simplejson.dump(packages, dst, indent=4)

+ 

+     def find_results_nevras_dicts(self):

+         """

+         Find all RPM packages in the `resultdir` and return their NEVRAs

+         as `dicts`

+         """

+         nevras = []

+         for result in os.listdir(self.resultdir):

+             if not result.endswith(".rpm"):

+                 continue

+             package = os.path.join(self.resultdir, result)

+             nevras.append(self.get_nevra_dict(package))

+         return nevras

+ 

+     @classmethod

+     def get_nevra_dict(cls, path):

+         """

+         Takes a package path and returns its NEVRA as a `dict`

+         """

+         filename = os.path.basename(path)

+         if not filename.endswith(".rpm"):

+             msg = "File name doesn't end with '.rpm': {}".format(path)

+             raise ValueError(msg)

+ 

+         hdr = cls.get_rpm_header(path)

+         arch = "src" if filename.endswith(".src.rpm") else hdr["arch"]

+         return {

+             "name": hdr["name"],

+             "epoch": hdr["epoch"] or 0,

+             "version": hdr["version"],

+             "release": hdr["release"],

+             "arch": arch,

+         }

+ 

+     @staticmethod

+     def get_rpm_header(path):

+         """

+         Examine a RPM package file and return its header

+         See docs.fedoraproject.org/en-US/Fedora_Draft_Documentation/0.1/html/RPM_Guide/ch16s04.html

+         """

+         ts = rpm.TransactionSet()

+         with open(path, "r") as f:

+             hdr = ts.hdrFromFdno(f.fileno())

+             return hdr

Metadata Update from @frostyx:
- Pull-request tagged with: wip

3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

This is WIP and a very early preview.

I want to discuss two questions.

  1. What format do we want to use? NVR, NEVRA, or something else?
  2. Do we want to inspect only RPM packages or SRPM packages as well?

I implemented the logic on the builder, as previously agreed upon. But I am struggling to find a proper way to get the information to the frontend. The best I could figure out was dumping a text file containing the results, downloading it on the backend, and then sending it to the frontend in _mark_finished request.

Insert the big brain time meme

I don't like the dumping&downloading a file idea very much. Can you think of a better solution, please?

What format do we want to use? NVR, NEVRA, or something else?

Reading the #1411 again, I think we want to go with NEVRA

rebased onto df228207e4fbf1b2b59f7225b8f88c81a219e853

3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

I implemented the first half of #1411.
So far, we create results.json on the builder, backend then downloads it as a part of the results, and then it is sent to frontend and stored into the database.

I would probably suggest ending the PR here and then continuing the API part in some follow-up PR. What do you think?

Or at least I will wait until this part is reviewed.

rebased onto 32cf7ebbc70cdf1bd98001b682d4d1c9b5a3ef48

3 years ago

Build succeeded.

rebased onto 3dbe2f5a052e01650aba4b85e3f067ff4737f2d3

3 years ago

Build succeeded.

rebased onto 19d3bd072d15783609f192a01cdff0ea88091090

3 years ago

Build succeeded.

rebased onto de0007f5eea7ff11604e5131977c0431a3f98018

3 years ago

Build succeeded.

I would probably suggest ending the PR here and then continuing the API part in some follow-up PR. What do you think?

Yes, sounds OK.

The task we do here is IMO easy enough to depend only on RPM. How to read the headers: http://ftp.rpm.org/api/4.4.2.2/classRpmhdr.html

Rpm can handle this:
$ rpm --qf "%{ARCH} + %{RELEASE}\n" -qp /tmp/quick-package/x86_64/dummy-pkg-20210422_0939-1.fc34.x86_64.rpm

But honestly, I looked at the get_nevra_possibilities() docs, and I don't think I understand what it is used for. Perhaps some in-line comment would help here.

This should be moved to _get_build_details. This is a risky place, any unhandled exception here could cause that _mark_finished() is not executed.

rebased onto a3ec05e6eadc87c95e75949b7e4c5de83b09c229

3 years ago

Metadata Update from @frostyx:
- Pull-request tagged with: needs-tests

3 years ago

Build succeeded.

rebased onto ca983798d4e1f933e1e0cf63e853d4c4b073ec3e

3 years ago

Build succeeded.

Backend build fails because of #1782

Metadata Update from @frostyx:
- Pull-request untagged with: wip

3 years ago

rebased onto e6c9ddbf586c9bb16c7c6481230502a84597539d

3 years ago

Metadata Update from @frostyx:
- Pull-request untagged with: needs-tests

3 years ago

Build succeeded.

Now when I see the format.. would you mind changing it to something like {"packages": [.. what you already have...]}? It will allow us to not change the format (and e.g. pg indexes) in the future when we wanted to add some additional info (e.g. reason for build failure, from top of my head).

Since we are not implementing the API yet - this stays unused for now. I'm curious whether we don't want to skip this for now and reconsider a separate table in the next release (maybe not, dunno). I mean, is the sa.JSON working with sqlite (our testsuite)?

rebased onto 0ecce3424d494d97f2e89835cb7edfb6af2143be

3 years ago

rebased onto b391ccbb38b3a49f684b7250708c1b085274acd8

3 years ago

Build succeeded.

Can you bump MIN_BUILDER_VERSION?

Done. Please check, I never know how to set the version properly. I am going to open an RFE regarding this.

would you mind changing it to something like {"packages":

Done

Since we are not implementing the API yet - this stays unused for now. I'm curious whether we don't want to skip this for now and reconsider a separate table in the next release (maybe not, dunno). I mean, is the sa.JSON working with sqlite (our testsuite)?

According to the documentation https://docs.sqlalchemy.org/en/13/core/type_basics.html?highlight=json#sqlalchemy.types.JSON ,
the support is as following

PostgreSQL
MySQL as of version 5.7 (MariaDB as of the 10.2 series does not)
SQLite as of version 3.9

So our testsuite is fine :-). We can postpone this PR until the next release if you want to, I don't mind. But "reconsider a separate table" I would really, relly prefer not doing this (yet). I would like to test the Json data type, it sounds like a cool feature but also, adding another table is not that small amount of work and IMHO there is no benefit since we will simply return the json as is. I know we can add some search based on NEVRA but I don't see it anytime soon and also, maybe the Json datatype will handle it well enough. I would like to try.

rebased onto fc3afd4da3d2fb76a4721e42f0283b86b69f46ee

3 years ago

Build succeeded.

I know we can add some search based on NEVRA but I don't see it anytime soon

I don't think it's anyhow complicated when we have the data? Dunno.

and also, maybe the Json datatype will handle it well enough. I would like to try.

I have nothing against experiments, just be warned that doing migrations on BuildChroot
is not trivial (large amounts of data).

The CI failure is not related, I guess?

Even though I'd prefer moving the database layout change to the other pull-request (the API addition, when we'll actually use that field) I think I'm +1, thank you (considering that the CI failure isn't related, and will be fixed in separate PR anyways).

rebased onto 0511df4c7a24d6adb63d742a7ca87beeb304140a

3 years ago

Build succeeded.

rebased onto ef7cb2fa30999d07173f4f869a67e720e4c05ca0

3 years ago

Build succeeded.

I am marking this as BLOCKED (by the upcoming release) so we don't accidentally merge.

Metadata Update from @frostyx:
- Pull-request tagged with: blocked

3 years ago

rebased onto 5b707818067a5ca6a2535271389d999da3bff255

3 years ago

Build succeeded.

3 new commits added

  • frontend: make sure we store an actual JSON not string
  • frontend, python: APIv3 support for querying built packages
  • frontend: add tests that it is possible to query based on results
3 years ago

I implemented the rest of the functionality, PTAL

Now when I see the format.. would you mind changing it to something like {"packages": [.. what you already have...]}?

I added "packages" but then I noticed that #1411 suggests "built_packages" so we can change it to whatever you prefer.

Metadata Update from @frostyx:
- Pull-request untagged with: blocked

3 years ago

Build succeeded.

There's some build failure, is it relevant to this PR?

Hm, I am a bit afraid that this will not scale (not with a very good indexing mechanism). We have roughly 3M of BuildChroot items ...
Something is telling me that we indeed should go with a separate table ... but let's talk about this on the meeting...

1 new commit added

  • frontend: store build results in separate table instead of JSON blob
3 years ago

Build succeeded.

I updated the PR so it uses a separate table instead of a JSON blob for storing built packages.

I for sure caused the build failure in F32 because I used a construct, that is available only in python3.9+. I can change it to something that is compatible with older python but I would rather just stop building Copr server packages for F32 anymore.

For the record, I used x | y for joining dictionaries instead of its ugly predecessor {**x, **y}.

I don't understand how this PR caused the FE failure in rawhide and BE failure in all chroots but I will look at it later.

I don't understand how this PR caused the FE failure in rawhide and BE failure in all chroots but I will look at it later.

So, it seems like BE failure is caused by me, I will fix it. But Frontend fails in rawhide even in main branch.

https://copr.fedorainfracloud.org/coprs/g/copr/copr-dev/build/2185085/

rebased onto e64425b743e4a4a11476dcc8bd524906e81743da

3 years ago

Build succeeded.

I fixed the backend tests, FE is still failing but please see my previous comments regarding that. Let me know before merging, I will squash some of the commits together. I left them this way, so you can review easily.

Here is an awesome opportunity to check the FE database / API as well.

If I get it right, we don't yet support searching by NEVRA (which is fine, we can do it later).
This PR looks really good, thank you for the progress!

2 new commits added

  • frontend: add forgotten migration
  • frontend: add more tests
3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

3 new commits added

  • beaker-tests-sanity: add tests for API endpoints
  • beaker-tests-sanity: fix whitespace
  • rpmbuild: dont skip SRPM files
3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

1 new commit added

  • beakerr-tests-sanity: not hardcode fc33 disttag
3 years ago

Can we keep source RPMs parsed?

Sure. Updated. But please note that now the results json simply contains the following record twice. Do we want to put some extra field in there to differentiate between them?

        {
            "arch": "x86_64",
            "epoch": 0,
            "name": "hello",
            "release": "1.fc33",
            "version": "2.8"
        },

Here is an awesome opportunity to check the FE database / API as well.

Sounds good, done

This will fail once we move to F34 (we should asap).

Good catch, fixed

If I get it right, we don't yet support searching by NEVRA (which is fine, we can do it later).

Exactly :-)

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

1 new commit added

  • frontend: fix zuul
3 years ago

Build succeeded.

Sure. Updated. But please note that now the results json simply contains
the following record twice. Do we want to put some extra field in there
to differentiate between them?

Interesting, I thought that RPM bakes an src string into %{ARCH} field
for source RPMs, but it is not like that (at least that's how DNF
differentiates between them..., see repoquery, and e.g. --archlist option,
... and I now checked that also createrepo also uses <arch>src</arch>).

That said, I have no better idea than just doing the same thing that
others do (reset "arch" to "src" for source RPMs).

1 new commit added

  • Rename arch for SRPM packages
3 years ago

Build succeeded.

That said, I have no better idea than just doing the same thing that
others do (reset "arch" to "src" for source RPMs).
See what createrepo_c does.

Good idea, thank you. Updated.

Aha, we don't have command line api for this functionality ATM. Sorry for pushing you this way .... pretty complicated test, but still good!

Nice, this PR is of very good quality. Thanks! Do you plan to squash some of those commits?

rebased onto 428df0457b9b1c4d50e9b132f831e6e1bb3f1d94

3 years ago

Build succeeded.

Sorry for pushing you this way .... pretty complicated test, but still good!

Np, I am glad we have those tests anyway. Sometimes I wish we had the beaker tests written in python, these kinds of tests would be much easier to write.

Do you plan to squash some of those commits?

I squashed everything into one commit.

rebased onto e481b0e1c646ef7bf420ede4dc5dae1c1375b36c

3 years ago

Build succeeded.

Still the gen_gpg_key action doesn't look like it needs the appstream argument.

Still the gen_gpg_key action doesn't look like it needs the appstream argument.

I guess this was a comment for PR#1722?

rebased onto b659056

3 years ago

Build succeeded.

Pull-Request has been merged by praiskup

3 years ago

@frostyx Thanks a lot for working on this! (It will really help us on the Packit side.)

Can we somehow track the deployment of this on copr.fedorainfracloud.org? (Or, which version of Copr itself and Python client we can expect this to lend in?)

This feature (as all others already merged) will be in the very next Copr release.
The release is not yet scheduled, but we should be able to wrap a new release in
the next 3 weeks.

@praiskup Nice, thanks for the quick response.