#382 Migrate from deprecated python3-modulemd (RhBug: 1609033)
Merged 5 years ago by clime. Opened 5 years ago by frostyx.
copr/ frostyx/copr libmodulemd  into  master

file modified
+13 -8
@@ -13,7 +13,9 @@ 

  from requests import RequestException

  from munch import Munch

  

- import modulemd

+ import gi

+ gi.require_version('Modulemd', '1.0')

+ from gi.repository import Modulemd

  

  from .sign import create_user_keys, CoprKeygenRequestError

  from .createrepo import createrepo
@@ -394,17 +396,19 @@ 

              ownername = data["ownername"]

              projectname = data["projectname"]

              chroots = data["chroots"]

-             modulemd_data = base64.b64decode(data["modulemd_b64"])

+             modulemd_data = base64.b64decode(data["modulemd_b64"]).decode("utf-8")

              project_path = os.path.join(self.opts.destdir, ownername, projectname)

              self.log.info(modulemd_data)

  

-             mmd = modulemd.ModuleMetadata()

-             mmd.loads(modulemd_data)

+             mmd = Modulemd.ModuleStream()

+             mmd.import_from_string(modulemd_data)

+             artifacts = Modulemd.SimpleSet()

  

              for chroot in chroots:

                  arch = get_chroot_arch(chroot)

                  srcdir = os.path.join(project_path, chroot)

-                 module_tag = chroot + '+' + mmd.name + '-' + (mmd.stream or '') + '-' + (str(mmd.version) or '1')

+                 module_tag = "{}+{}-{}-{}".format(chroot, mmd.get_name(), (mmd.get_stream() or ''),

+                                                   (str(mmd.get_version()) or '1'))

                  module_relpath = os.path.join(module_tag, "latest", arch)

                  destdir = os.path.join(project_path, "modules", module_relpath)

  
@@ -424,10 +428,11 @@ 

                          for f in os.listdir(os.path.join(destdir, folder)):

                              if not f.endswith(".rpm") or f.endswith(".src.rpm"):

                                  continue

-                             mmd.artifacts.rpms.add(str(f.rstrip(".rpm")))

+                             artifacts.add(str(f.rstrip(".rpm")))

  

-                     self.log.info("Module artifacts: %s", mmd.artifacts.rpms)

-                     modulemd.dump_all(os.path.join(destdir, "modules.yaml"), [mmd])

+                     mmd.set_rpm_artifacts(artifacts)

+                     self.log.info("Module artifacts: %s", mmd.get_rpm_artifacts())

+                     Modulemd.dump([mmd], os.path.join(destdir, "modules.yaml"))

                      createrepo(path=destdir, front_url=self.front_url,

                                 username=ownername, projectname=projectname,

                                 override_acr_flag=True)

@@ -5,7 +5,9 @@ 

  

  from backend.vm_manage import PUBSUB_INTERRUPT_BUILDER

  

- import modulemd

+ import gi

+ gi.require_version('Modulemd', '1.0')

+ from gi.repository import Modulemd

  

  from ..helpers import get_redis_connection, ensure_dir_exists

  from ..exceptions import BuilderError, RemoteCmdError, VmError
@@ -48,9 +50,9 @@ 

      def _load_module_dist_tag(self):

          module_md_filepath = os.path.join(self.job.destdir, self.job.chroot, "module_md.yaml")

          try:

-             mmd = modulemd.ModuleMetadata()

-             mmd.load(module_md_filepath)

-             dist_tag = ("." + mmd.name + '+' + mmd.stream + '+' + str(mmd.version))

+             mmd = Modulemd.ModuleStream()

+             mmd.import_from_file(module_md_filepath)

+             dist_tag = ".{}+{}+{}".format(mmd.get_name(), (mmd.get_stream() or ''), (str(mmd.get_version()) or '1'))

          except IOError as e:

              return None

          except Exception as e:

file modified
+2 -1
@@ -60,7 +60,8 @@ 

  Requires:   python3-dateutil

  Requires:   python3-pytz

  Requires:   python3-netaddr

- Requires:   python3-modulemd

+ Requires:   python3-gobject

+ Requires:   libmodulemd

  Requires:   python3-configparser

  Requires:   python3-fedmsg

  Requires:   redis

file modified
+4 -1
@@ -92,7 +92,8 @@ 

  BuildRequires: python3-CommonMark

  BuildRequires: python3-pygments

  BuildRequires: python3-flask-whooshee

- BuildRequires: python3-modulemd

+ BuildRequires: python3-gobject

+ BuildRequires: libmodulemd

  BuildRequires: python3-requests

  BuildRequires: redis

  %endif
@@ -125,6 +126,8 @@ 

  Requires: python3-flask-openid

  Requires: python3-openid-teams

  Requires: python3-modulemd

+ Requires: python3-gobject

+ Requires: libmodulemd

  Requires: python3-pygments

  Requires: python3-CommonMark

  Requires: python3-psycopg2

@@ -3,7 +3,6 @@ 

  import base64

  import json

  import requests

- import modulemd

  from collections import defaultdict

  from sqlalchemy import and_

  from datetime import datetime
@@ -13,6 +12,10 @@ 

  from coprs.logic import builds_logic

  from wtforms import ValidationError

  

+ import gi

+ gi.require_version('Modulemd', '1.0')

+ from gi.repository import Modulemd

+ 

  

  class ModulesLogic(object):

      @classmethod
@@ -45,19 +48,19 @@ 

  

      @classmethod

      def yaml2modulemd(cls, yaml):

-         mmd = modulemd.ModuleMetadata()

-         mmd.loads(yaml)

+         mmd = Modulemd.ModuleStream()

+         mmd.import_from_string(yaml)

          return mmd

  

      @classmethod

      def from_modulemd(cls, mmd):

          yaml_b64 = base64.b64encode(mmd.dumps().encode("utf-8")).decode("utf-8")

-         return models.Module(name=mmd.name, stream=mmd.stream, version=mmd.version, summary=mmd.summary,

-                              description=mmd.description, yaml_b64=yaml_b64)

+         return models.Module(name=mmd.get_name(), stream=mmd.get_stream(), version=mmd.get_version(),

+                              summary=mmd.get_summary(), description=mmd.get_description(), yaml_b64=yaml_b64)

  

      @classmethod

      def validate(cls, mmd):

-         if not all([mmd.name, mmd.stream, mmd.version]):

+         if not all([mmd.get_name(), mmd.get_stream(), mmd.get_version()]):

              raise ValidationError("Module should contain name, stream and version")

  

      @classmethod
@@ -74,9 +77,9 @@ 

  

      @classmethod

      def set_defaults_for_optional_params(cls, mmd, filename=None):

-         mmd.name = mmd.name or str(os.path.splitext(filename)[0])

-         mmd.stream = mmd.stream or "master"

-         mmd.version = mmd.version or int(datetime.now().strftime("%Y%m%d%H%M%S"))

+         mmd.set_name(mmd.get_name() or str(os.path.splitext(filename)[0]))

+         mmd.set_stream(mmd.get_stream() or "master")

+         mmd.set_version(mmd.get_version() or int(datetime.now().strftime("%Y%m%d%H%M%S")))

  

  

  class ModuleBuildFacade(object):
@@ -92,7 +95,7 @@ 

  

      def submit_build(self):

          module = ModulesLogic.add(self.user, self.copr, ModulesLogic.from_modulemd(self.modulemd))

-         self.add_builds(self.modulemd.components.rpms, module)

+         self.add_builds(self.modulemd.get_rpm_components(), module)

          return module

  

      @classmethod
@@ -105,7 +108,7 @@ 

          """

          batches = defaultdict(dict)

          for pkgname, rpm in rpms.items():

-             batches[rpm.buildorder][pkgname] = rpm

+             batches[rpm.get_buildorder()][pkgname] = rpm

          return [batches[number] for number in sorted(batches.keys())]

  

      def add_builds(self, rpms, module):
@@ -115,14 +118,16 @@ 

              for pkgname, rpm in group.items():

                  clone_url = self.get_clone_url(pkgname, rpm)

                  build = builds_logic.BuildsLogic.create_new_from_scm(self.user, self.copr, scm_type="git",

-                                                                      clone_url=clone_url, committish=rpm.ref)

+                                                                      clone_url=clone_url, committish=rpm.get_ref())

                  build.batch = batch

                  build.batch_id = batch.id

                  build.module_id = module.id

                  db.session.add(build)

  

      def get_clone_url(self, pkgname, rpm):

-         return rpm.repository if rpm.repository else self.default_distgit.format(pkgname=pkgname)

+         if rpm.get_repository():

+             return rpm.get_repository()

+         return self.default_distgit.format(pkgname=pkgname)

  

      @property

      def default_distgit(self):
@@ -133,30 +138,34 @@ 

  class ModulemdGenerator(object):

      def __init__(self, name="", stream="", version=0, summary="", config=None):

          self.config = config

-         self.mmd = modulemd.ModuleMetadata()

-         self.mmd.name = name

-         self.mmd.stream = stream

-         self.mmd.version = version

-         self.mmd.summary = summary

+         licenses = Modulemd.SimpleSet()

+         licenses.add("unknown")

+         self.mmd = Modulemd.ModuleStream(mdversion=1, name=name, stream=stream, version=version, summary=summary,

+                                          description="", content_licenses=licenses, module_licenses=licenses)

  

      @property

      def nsv(self):

-         return "{}-{}-{}".format(self.mmd.name, self.mmd.stream, self.mmd.version)

+         return "{}-{}-{}".format(self.mmd.get_name(), self.mmd.get_stream(), self.mmd.get_version())

  

      def add_api(self, packages):

+         mmd_set = Modulemd.SimpleSet()

          for package in packages:

-             self.mmd.api.add_rpm(str(package))

+             mmd_set.add(str(package))

+         self.mmd.set_rpm_api(mmd_set)

  

      def add_filter(self, packages):

+         mmd_set = Modulemd.SimpleSet()

          for package in packages:

-             self.mmd.filter.add_rpm(str(package))

+             mmd_set.add(str(package))

+         self.mmd.set_rpm_filter(mmd_set)

  

      def add_profiles(self, profiles):

          for i, values in profiles:

              name, packages = values

-             self.mmd.profiles[name] = modulemd.profile.ModuleProfile()

+             profile = Modulemd.Profile(name=name)

              for package in packages:

-                 self.mmd.profiles[name].add_rpm(str(package))

+                 profile.add_rpm(str(package))

+             self.mmd.add_profile(profile)

  

      def add_components(self, packages, filter_packages, builds):

          build_ids = sorted(list(set([int(id) for p, id in zip(packages, builds)
@@ -180,57 +189,13 @@ 

          ref = str(chroot.git_hash) if chroot else ""

          distgit_url = self.config["DIST_GIT_URL"].replace("/cgit", "/git")

          url = os.path.join(distgit_url, build.copr.full_name, "{}.git".format(build.package.name))

-         self.mmd.components.add_rpm(str(package_name), rationale,

-                                     repository=url, ref=ref,

-                                     buildorder=buildorder)

- 

-     def add_requires(self, module, stream):

-         self.mmd.add_requires(module, stream)

- 

-     def add_buildrequires(self, module, stream):

-         self.mmd.add_buildrequires(module, stream)

+         component = Modulemd.ComponentRpm(name=str(package_name), rationale=rationale,

+                                           repository=url, ref=ref, buildorder=1)

+         self.mmd.add_rpm_component(component)

  

      def generate(self):

          return self.mmd.dumps()

  

-     def dump(self, handle):

-         return self.mmd.dump(handle)

- 

- 

- class MBSProxy(object):

-     def __init__(self, mbs_url, user_name=None):

-         self.url = mbs_url

-         self.user = user_name

- 

-     def post(self, json=None, data=None, files=None):

-         request = requests.post(self.url, verify=False,

-                                 json=json, data=data, files=files)

-         return MBSResponse(request)

- 

-     def build_module(self, owner, project, nsv, modulemd):

-         return self.post(

-             data={"owner": self.user, "copr_owner": owner, "copr_project": project},

-             files={"yaml": ("{}.yaml".format(nsv), modulemd)},

-         )

- 

- 

- class MBSResponse(object):

-     def __init__(self, response):

-         self.response = response

- 

-     @property

-     def failed(self):

-         return self.response.status_code != 201

- 

-     @property

-     def message(self):

-         if self.response.status_code in [500, 403, 404]:

-             return "Error from MBS: {} - {}".format(self.response.status_code, self.response.reason)

-         resp = json.loads(self.response.content)

-         if self.response.status_code != 201:

-             return "Error from MBS: {}".format(resp["message"])

-         return "Created module {}-{}-{}".format(resp["name"], resp["stream"], resp["version"])

- 

  

  class ModuleProvider(object):

      def __init__(self, filename, yaml):

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

  import flask

  import json

  import base64

- import modulemd

  import uuid

  

  from sqlalchemy.ext.associationproxy import association_proxy
@@ -21,6 +20,10 @@ 

  import operator

  from coprs.helpers import BuildSourceEnum, StatusEnum, ActionTypeEnum, JSONEncodedDict

  

+ import gi

+ gi.require_version('Modulemd', '1.0')

+ from gi.repository import Modulemd

+ 

  

  class CoprSearchRelatedData(object):

      def get_search_related_copr_id(self):
@@ -1288,8 +1291,8 @@ 

  

      @property

      def modulemd(self):

-         mmd = modulemd.ModuleMetadata()

-         mmd.loads(self.yaml)

+         mmd = Modulemd.ModuleStream()

+         mmd.import_from_string(self.yaml.decode("utf-8"))

          return mmd

  

      @property
@@ -1320,6 +1323,18 @@ 

          """

          return helpers.ModuleStatusEnum(self.status)

  

+     @property

+     def rpm_filter(self):

+         return self.modulemd.get_rpm_filter().get()

+ 

+     @property

+     def rpm_api(self):

+         return self.modulemd.get_rpm_api().get()

+ 

+     @property

+     def profiles(self):

+         return {k: v.get_rpms().get() for k, v in self.modulemd.get_profiles().items()}

+ 

  

  class BuildsStatistics(db.Model):

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

@@ -64,7 +64,7 @@ 

        </div>

        <div class="panel-body">

          <ul>

-           {% for package in module.modulemd.filter.rpms %}

+           {% for package in module.rpm_filter %}

              <li>{{ package }}</li>

            {% else %}

              <p>{{ no_packages | format('filter')}}</p>
@@ -81,7 +81,7 @@ 

        </div>

        <div class="panel-body">

          <ul>

-         {% for package in module.modulemd.api.rpms %}

+         {% for package in module.rpm_api %}

            <li>{{ package }}</li>

          {% else %}

            <p>{{ no_packages | format('API')}}</p>
@@ -98,10 +98,10 @@ 

        </div>

        <div class="panel-body">

          <ul>

-           {% for name, profile in module.modulemd.profiles.items() %}

+           {% for name, packages in module.profiles.items() %}

              <li>{{ name }}</li>

              <ul>

-               {% for package in profile.rpms %}

+               {% for package in packages %}

                  <li>{{ package }}</li>

                {% endfor %}

              </ul>

@@ -15,7 +15,6 @@ 

  import smtplib

  import tempfile

  import sqlalchemy

- import modulemd

  from email.mime.text import MIMEText

  from itertools import groupby

  from wtforms import ValidationError

@@ -3,17 +3,21 @@ 

  from unittest import mock

  

  from tests.coprs_test_case import CoprsTestCase

- from coprs.logic.modules_logic import ModuleBuildFacade, ModulemdGenerator, MBSResponse, MBSProxy

- from modulemd.components.rpm import ModuleComponentRPM

+ from coprs.logic.modules_logic import ModuleBuildFacade, ModulemdGenerator

+ 

+ import gi

+ gi.require_version('Modulemd', '1.0')

+ from gi.repository import Modulemd

  

  

  class TestModuleBuildFacade(CoprsTestCase):

      def test_get_build_batches(self):

-         pkg1 = ModuleComponentRPM("pkg1", "rationale")

-         pkg2 = ModuleComponentRPM("pkg2", "rationale")

-         pkg3 = ModuleComponentRPM("pkg3", "rationale", buildorder=1)

-         pkg4 = ModuleComponentRPM("pkg4", "rationale", buildorder=-20)

-         pkg5 = ModuleComponentRPM("pkg5", "rationale", buildorder=50)

+         pkg1 = Modulemd.ComponentRpm(name="pkg1", rationale="rationale")

+         pkg2 = Modulemd.ComponentRpm(name="pkg2", rationale="rationale")

+         pkg3 = Modulemd.ComponentRpm(name="pkg3", rationale="rationale", buildorder=1)

+         pkg4 = Modulemd.ComponentRpm(name="pkg4", rationale="rationale")

+         pkg4.set_buildorder(-20) # https://github.com/fedora-modularity/libmodulemd/issues/77#issuecomment-418198410

+         pkg5 = Modulemd.ComponentRpm(name="pkg5", rationale="rationale", buildorder=50)

  

          # Test trivial usage

          assert ModuleBuildFacade.get_build_batches({}) == []
@@ -43,24 +47,25 @@ 

      config = {"DIST_GIT_URL": "http://distgiturl.org"}

  

      def test_basic_mmd_attrs(self):

-         generator = ModulemdGenerator("testmodule", "master", 123, "Some testmodule summary", None)

-         assert generator.mmd.name == "testmodule"

-         assert generator.mmd.stream == "master"

-         assert generator.mmd.version == 123

-         assert generator.mmd.summary == "Some testmodule summary"

+         generator = ModulemdGenerator(name="testmodule", stream="master",

+                                       version=123, summary="Some testmodule summary")

+         assert generator.mmd.get_name() == "testmodule"

+         assert generator.mmd.get_stream() == "master"

+         assert generator.mmd.get_version() == 123

+         assert generator.mmd.get_summary() == "Some testmodule summary"

          assert generator.nsv == "testmodule-master-123"

  

      def test_api(self):

          packages = {"foo", "bar", "baz"}

          generator = ModulemdGenerator()

          generator.add_api(packages)

-         assert generator.mmd.api.rpms == packages

+         assert set(generator.mmd.get_rpm_api().get()) == packages

  

      def test_filter(self):

          packages = {"foo", "bar", "baz"}

          generator = ModulemdGenerator()

          generator.add_filter(packages)

-         assert generator.mmd.filter.rpms == packages

+         assert set(generator.mmd.get_rpm_filter().get()) == packages

  

      def test_profiles(self):

          profile_names = ["default", "debug"]
@@ -68,28 +73,28 @@ 

          profiles = enumerate(zip(profile_names, profile_pkgs))

          generator = ModulemdGenerator()

          generator.add_profiles(profiles)

-         assert set(generator.mmd.profiles.keys()) == set(profile_names)

-         assert generator.mmd.profiles["default"].rpms == {"pkg1", "pkg2"}

-         assert generator.mmd.profiles["debug"].rpms == {"pkg3"}

-         assert len(generator.mmd.profiles) == 2

+         assert set(generator.mmd.get_profiles().keys()) == set(profile_names)

+         assert set(generator.mmd.get_profiles()["default"].get_rpms().get()) == {"pkg1", "pkg2"}

+         assert set(generator.mmd.get_profiles()["debug"].get_rpms().get()) == {"pkg3"}

+         assert len(generator.mmd.get_profiles()) == 2

  

      def test_add_component(self, f_users, f_coprs, f_mock_chroots, f_builds, f_db):

          chroot = self.b1.build_chroots[0]

          generator = ModulemdGenerator(config=self.config)

          generator.add_component(self.b1.package_name, self.b1, chroot,

                                  "A reason why package is in the module", buildorder=1)

-         assert len(generator.mmd.components.rpms) == 1

+         assert len(generator.mmd.get_rpm_components()) == 1

  

-         component = generator.mmd.components.rpms[self.b1.package_name]

-         assert component.buildorder == 1

-         assert component.name == self.b1.package_name

-         assert component.rationale == "A reason why package is in the module"

+         component = generator.mmd.get_rpm_components()[self.b1.package_name]

+         assert component.get_buildorder() == 1

+         assert component.get_name() == self.b1.package_name

+         assert component.get_rationale() == "A reason why package is in the module"

  

          with mock.patch("coprs.app.config", self.config):

-             assert component.repository.startswith("http://distgiturl.org")

-             assert component.repository.endswith(".git")

-             assert chroot.dist_git_url.startswith(component.repository)

-         assert component.ref == chroot.git_hash

+             assert component.get_repository().startswith("http://distgiturl.org")

+             assert component.get_repository().endswith(".git")

+             assert chroot.dist_git_url.startswith(component.get_repository())

+         assert component.get_ref() == chroot.git_hash

  

      def test_components(self, f_users, f_coprs, f_mock_chroots, f_builds, f_db):

          packages = [self.p1.name, self.p2.name, self.p3.name]
@@ -122,37 +127,3 @@ 

          generator.add_api(["foo", "bar", "baz"])

          generator.add_filter(["foo", "bar"])

          yaml.load(generator.generate())

- 

- 

- class TestMBSResponse(CoprsTestCase):

-     def test_status(self):

-         assert MBSResponse(Munch(status_code=500)).failed is True

-         assert MBSResponse(Munch(status_code=409)).failed is True

-         assert MBSResponse(Munch(status_code=201)).failed is False

- 

-     def test_message(self):

-         req1 = Munch(status_code=500, reason="foo reason")

-         res1 = MBSResponse(req1)

-         assert res1.message == "Error from MBS: 500 - foo reason"

- 

-         req2 = Munch(status_code=409, content='{"message": "foo message"}')

-         res2 = MBSResponse(req2)

-         assert res2.message == "Error from MBS: foo message"

- 

-         con3 = '{"name": "testmodule", "stream": "master", "version": 123}'

-         req3 = Munch(status_code=201, content=con3)

-         res3 = MBSResponse(req3)

-         assert res3.message == "Created module testmodule-master-123"

- 

- 

- class TestMBSProxy(CoprsTestCase):

- 

-     @mock.patch("requests.post")

-     def test_post(self, post_mock):

-         url = "http://some-module-build-service.org"

-         proxy = MBSProxy(url)

-         response = proxy.post(None, None, None)

-         post_mock.assert_called()

-         args, kwargs = post_mock.call_args

-         assert args[0] == url

-         assert isinstance(response, MBSResponse)

This PR migrates both frontend and backend code from using deprecated python3-modulemd to libmodulemd.
Since the libmodulemd is a C library, its usage doesn't feel as pythonic as I would like to. Ideally it would be nice to wrap its code in some adapter class, but it is not that straightforward (Is it worth it? How to use it on both FE and BE? Do it in Copr code, or create a new package, so people can use it?), therefore I decided to do a 1:1 changes, so we can just solve the BZ and be able to build Copr packages for rawhide again. Additional improvements may come in the future.

LGTM. Please, reasolve the conflicts.

+1, testsuite passes, can we please rebase and merge this?

Ideally it would be nice to wrap its code in some adapter class, but it is not that straightforward (Is it worth it? How to use it on both FE and BE? Do it in Copr code, or create a new package, so people can use it?), therefore I decided to do a 1:1 changes, so we can just solve the BZ and be able to build Copr packages for rawhide again.

These problems are is not supposed to be solved by copr team IMO, but bug against modularity team would be good. The 1:1 approach is fine.

rebased onto e28b501

5 years ago

I've rebased the branch. There was a change in migration, but the migration file no longer exists after bc9328c, so I've removed it.

Pull-Request has been merged by clime

5 years ago