#186 Modularity is Dead, Long Live Modularity!
Merged 6 years ago by frostyx. Opened 6 years ago by frostyx.

file modified
+10 -2
@@ -442,8 +442,16 @@ 

                  if os.path.exists(destdir):

                      self.log.warning("Module {0} already exists. Omitting.".format(destdir))

                  else:

-                     self.log.info("Copy directory: {} as {}".format(srcdir, destdir))

-                     shutil.copytree(srcdir, destdir)

+                     # We want to copy just the particular module builds

+                     # into the module destdir, not the whole chroot

+                     os.makedirs(destdir)

+                     prefixes = ["{:08d}-".format(x) for x in data["builds"]]

+                     dirs = [d for d in os.listdir(srcdir) if d.startswith(tuple(prefixes))]

+                     for folder in dirs:

+                         shutil.copytree(os.path.join(srcdir, folder), os.path.join(destdir, folder))

+                         self.log.info("Copy directory: {} as {}".format(

+                             os.path.join(srcdir, folder), os.path.join(destdir, folder)))

+ 

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

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

                                 username=ownername, projectname=projectname,

file modified
+2 -2
@@ -621,7 +621,7 @@ 

          """

          Build module via Copr MBS

          """

-         ownername, projectname = parse_name(args.copr or "")

+         ownername, projectname = parse_name(args.copr)

          modulemd = open(args.yaml, "rb") if args.yaml else args.url

          response = self.client.build_module(modulemd, ownername, projectname)

          print(response.message if response.output == "ok" else response.error)
@@ -1057,7 +1057,7 @@ 

  

      # module building

      parser_build_module = subparsers.add_parser("build-module", help="Builds a given module in Copr")

-     parser_build_module.add_argument("copr", help="The copr repo to list the packages of. Can be just name of project or even in format owner/project.", nargs="?")

+     parser_build_module.add_argument("copr", help="The copr repo to build module in. Can be just name of project or even in format owner/project.")

      parser_build_module_mmd_source = parser_build_module.add_mutually_exclusive_group(required=True)

      parser_build_module_mmd_source.add_argument("--url", help="SCM with modulemd file in yaml format")

      parser_build_module_mmd_source.add_argument("--yaml", help="Path to modulemd file in yaml format")

@@ -0,0 +1,22 @@ 

+ """Change module version to bigint

+ 

+ Revision ID: 1f94b22f70a1

+ Revises: f61a5c930abf

+ Create Date: 2017-12-29 22:22:09.256634

+ 

+ """

+ 

+ # revision identifiers, used by Alembic.

+ revision = '1f94b22f70a1'

+ down_revision = 'f61a5c930abf'

+ 

+ from alembic import op

+ import sqlalchemy as sa

+ 

+ 

+ def upgrade():

+     op.alter_column('module', 'version', existing_type=sa.Integer(), type_=sa.BigInteger())

+ 

+ 

+ def downgrade():

+     op.alter_column('module', 'version', existing_type=sa.BigInteger(), type_=sa.Integer())

@@ -0,0 +1,24 @@ 

+ """Add build to module relation

+ 

+ Revision ID: 3b0851cb25fc

+ Revises: 7bb0c7762df0

+ Create Date: 2017-12-27 13:46:07.774167

+ 

+ """

+ 

+ # revision identifiers, used by Alembic.

+ revision = '3b0851cb25fc'

+ down_revision = '7bb0c7762df0'

+ 

+ from alembic import op

+ import sqlalchemy as sa

+ 

+ 

+ def upgrade():

+     op.add_column('build', sa.Column('module_id', sa.Integer(), nullable=True))

+     op.create_foreign_key(None, 'build', 'module', ['module_id'], ['id'])

This columnt should have index. Foreign key is integrity constraint, it is not used for speed up queries.

+ 

+ 

+ def downgrade():

+     op.drop_constraint(None, 'build', type_='foreignkey')

+     op.drop_column('build', 'module_id')

@@ -0,0 +1,22 @@ 

+ """Add index to module_id

+ 

+ Revision ID: e183e12563ee

+ Revises: 1f94b22f70a1

+ Create Date: 2018-01-07 11:03:29.885276

+ 

+ """

+ 

+ # revision identifiers, used by Alembic.

+ revision = 'e183e12563ee'

+ down_revision = '1f94b22f70a1'

+ 

+ from alembic import op

+ import sqlalchemy as sa

+ 

+ 

+ def upgrade():

+     op.create_index(op.f('ix_build_module_id'), 'build', ['module_id'], unique=False)

+ 

+ 

+ def downgrade():

+     op.drop_index(op.f('ix_build_module_id'), table_name='build')

@@ -0,0 +1,22 @@ 

+ """Unique constraint on modules

+ 

+ Revision ID: f61a5c930abf

+ Revises: 3b0851cb25fc

+ Create Date: 2017-12-27 16:32:51.786669

+ 

+ """

+ 

+ # revision identifiers, used by Alembic.

+ revision = 'f61a5c930abf'

+ down_revision = '3b0851cb25fc'

+ 

+ from alembic import op

+ import sqlalchemy as sa

+ 

+ 

+ def upgrade():

+     op.create_unique_constraint('copr_name_stream_version_uniq', 'module', ['copr_id', 'name', 'stream', 'version'])

+ 

+ 

+ def downgrade():

+     op.drop_constraint('copr_name_stream_version_uniq', 'module', type_='unique')

@@ -704,8 +704,6 @@ 

      modulemd = FileField("modulemd")

      scmurl = wtforms.StringField()

      branch = wtforms.StringField()

-     copr_owner = wtforms.StringField()

-     copr_project = wtforms.StringField()

  

  

  class ChrootForm(FlaskForm):
@@ -883,9 +881,6 @@ 

  

  

  class CreateModuleForm(FlaskForm):

-     name = wtforms.StringField("Name")

-     stream = wtforms.StringField("Stream")

-     version = wtforms.IntegerField("Version")

      builds = wtforms.FieldList(wtforms.StringField("Builds ID list"))

      packages = wtforms.FieldList(wtforms.StringField("Packages list"))

      filter = wtforms.FieldList(wtforms.StringField("Package Filter"))
@@ -901,12 +896,6 @@ 

          if not FlaskForm.validate(self):

              return False

  

-         module = ModulesLogic.get_by_nsv(self.copr, self.name.data, self.stream.data, self.version.data).first()

-         if module:

-             self.errors["nsv"] = [Markup("Module <a href='{}'>{}</a> already exists".format(

-                 helpers.copr_url("coprs_ns.copr_module", module.copr, id=module.id), module.full_name))]

-             return False

- 

          # Profile names should be unique

          names = filter(None, self.profile_names.data)

          if len(set(names)) < len(names):

@@ -10,6 +10,7 @@ 

  import re

  

  import flask

+ import posixpath

  from flask import url_for

  from dateutil import parser as dt_parser

  from netaddr import IPAddress, IPNetwork
@@ -293,7 +294,7 @@ 

          if mock_chroot.os_version != "rawhide":

              os_version = "$releasever"

  

-     url = urljoin(

+     url = posixpath.join(

          url, "{0}-{1}-{2}/".format(mock_chroot.os_release,

                                     os_version, "$basearch"))

  

@@ -257,17 +257,15 @@ 

          db.session.add(action)

  

      @classmethod

-     def send_build_module(cls, user, copr, module):

+     def send_build_module(cls, copr, module):

          """

          :type copr: models.Copr

          :type modulemd: str content of module yaml file

          """

  

-         if not user.can_build_in(copr):

-             raise exceptions.InsufficientRightsException("You don't have permissions to build in this copr.")

- 

          data = {

              "chroots": [c.name for c in copr.active_chroots],

+             "builds": [b.id for b in module.builds],

          }

  

          action = models.Action(

@@ -758,6 +758,13 @@ 

  

                      db.session.add(build_chroot)

  

+                     # If the last package of a module was successfully built,

+                     # then send an action to create module repodata on backend

+                     if (build.module

+                             and upd_dict.get("status") == StatusEnum("succeeded")

+                             and all(b.status == StatusEnum("succeeded") for b in build.module.builds)):

Generally, it is better to rewrite it to exists() rather than all() as it is faster. E.g., ... and not exists(b.status != StatusEnum("succeeded") for b in build.module.builds)

+                         ActionsLogic.send_build_module(build.copr, build.module)

+ 

          for attr in ["results", "built_packages", "srpm_url"]:

              value = upd_dict.get(attr, None)

              if value:

@@ -5,6 +5,7 @@ 

  import requests

  import modulemd

  from sqlalchemy import and_

+ from datetime import datetime

  from coprs import models

  from coprs import db

  from coprs import exceptions
@@ -29,6 +30,11 @@ 

                   models.Module.copr_id == copr.id))

  

      @classmethod

+     def get_by_nsv_str(cls, copr, nsv):

+         name, stream, version = nsv.split("-")

+         return cls.get_by_nsv(copr, name, stream, version)

+ 

+     @classmethod

      def get_multiple(cls):

          return models.Module.query.order_by(models.Module.id.desc())

  
@@ -43,14 +49,13 @@ 

          return mmd

  

      @classmethod

-     def from_modulemd(cls, yaml):

-         mmd = cls.yaml2modulemd(yaml)

+     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=base64.b64encode(yaml))

+                              description=mmd.description, yaml_b64=yaml_b64)

  

      @classmethod

-     def validate(cls, yaml):

-         mmd = cls.yaml2modulemd(yaml)

+     def validate(cls, mmd):

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

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

  
@@ -66,6 +71,57 @@ 

          db.session.add(module)

          return module

  

+     @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"))

+ 

+ 

+ class ModuleBuildFacade(object):

+     def __init__(self, user, copr, yaml, filename=None):

+         self.user = user

+         self.copr = copr

+         self.yaml = yaml

+         self.filename = filename

+ 

+         self.modulemd = ModulesLogic.yaml2modulemd(yaml)

+         ModulesLogic.set_defaults_for_optional_params(self.modulemd, filename=filename)

+         ModulesLogic.validate(self.modulemd)

+ 

+     def submit_build(self):

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

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

+         return module

+ 

+     def get_build_batches(self, rpms):

+         """

+         Determines Which component should be built in which batch. Returns an ordered list of grouped components,

+         first group of components should be built as a first batch, second as second and so on.

+         Particular components groups are represented by lists and can by built in a random order within the batch.

+         :return: list of lists

+         """

+         return [rpms]

+ 

+     def add_builds(self, rpms, module):

+         for group in self.get_build_batches(rpms):

+             batch = models.Batch()

+             db.session.add(batch)

+             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)

+                 build.batch_id = batch.id

+                 build.module_id = module.id

+ 

+     def get_clone_url(self, pkgname, rpm):

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

+ 

+     @property

+     def default_distgit(self):

+         # @TODO move to better place

+         return "https://src.fedoraproject.org/rpms/{pkgname}"

+ 

  

  class ModulemdGenerator(object):

      def __init__(self, name="", stream="", version=0, summary="", config=None):
@@ -127,11 +183,6 @@ 

      def add_buildrequires(self, module, stream):

          self.mmd.add_buildrequires(module, stream)

  

-     def add_base_runtime(self):

-         name, stream = "base-runtime", "master"

-         self.add_requires(name, stream)

-         self.add_buildrequires(name, stream)

- 

      def generate(self):

          return self.mmd.dumps()

  
@@ -172,3 +223,29 @@ 

          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):

+         self.filename = filename

+         self.yaml = yaml

+ 

+     @classmethod

+     def from_input(cls, obj):

+         if hasattr(obj, "read"):

+             return cls.from_file(obj)

+         return cls.from_url(obj)

+ 

+     @classmethod

+     def from_file(cls, ref):

+         return cls(ref.filename, ref.read())

+ 

+     @classmethod

+     def from_url(cls, url):

+         if not url.endswith(".yaml"):

+             raise ValidationError("This URL doesn't point to a .yaml file")

+ 

+         request = requests.get(url)

+         if request.status_code != 200:

+             raise requests.RequestException("This URL seems to be wrong")

+         return cls(os.path.basename(url), request.text)

@@ -553,6 +553,9 @@ 

      batch_id = db.Column(db.Integer, db.ForeignKey("batch.id"))

      batch = db.relationship("Batch", backref=db.backref("builds"))

  

+     module_id = db.Column(db.Integer, db.ForeignKey("module.id"), index=True)

+     module = db.relationship("Module", backref=db.backref("builds"))

+ 

      @property

      def user_name(self):

          return self.user.name
@@ -1205,7 +1208,7 @@ 

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

      name = db.Column(db.String(100), nullable=False)

      stream = db.Column(db.String(100), nullable=False)

-     version = db.Column(db.Integer, nullable=False)

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

      summary = db.Column(db.String(100), nullable=False)

      description = db.Column(db.Text)

      created_on = db.Column(db.Integer, nullable=True)
@@ -1222,6 +1225,10 @@ 

      copr_id = db.Column(db.Integer, db.ForeignKey("copr.id"))

      copr = db.relationship("Copr", backref=db.backref("modules"))

  

+     __table_args__ = (

+         db.UniqueConstraint("copr_id", "name", "stream", "version", name="copr_name_stream_version_uniq"),

+     )

+ 

      @property

      def yaml(self):

          return base64.b64decode(self.yaml_b64)
@@ -1245,17 +1252,17 @@ 

          return Action.query.filter(Action.object_type == "module").filter(Action.object_id == self.id).first()

  

      @property

+     def status(self):

+         """

+         Return numeric representation of status of this build

+         """

+         if any(b for b in self.builds if b.status == StatusEnum("failed")):

+             return helpers.ModuleStatusEnum("failed")

+         return self.action.result if self.action else helpers.ModuleStatusEnum("pending")

+ 

+     @property

      def state(self):

          """

          Return text representation of status of this build

          """

-         if self.action is not None:

-             return helpers.ModuleStatusEnum(self.action.result)

-         return "-"

- 

-     def repo_url(self, arch):

-         # @TODO Use custom chroot instead of fedora-24

-         # @TODO Get rid of OS name from module path, see how koji does it

-         # https://kojipkgs.stg.fedoraproject.org/repos/module-base-runtime-0.25-9/latest/x86_64/toplink/packages/module-build-macros/0.1/

-         module_dir = "fedora-24-{}+{}-{}-{}".format(arch, self.name, self.stream, self.version)

-         return "/".join([self.copr.repo_url, "modules", module_dir, "latest", arch])

+         return helpers.ModuleStatusEnum(self.status)

@@ -135,11 +135,7 @@ 

  {% endmacro %}

  

  {% macro module_state(module) %}

-   {% if module.action %}

-     {{ build_state_text(module.action.result | module_state_from_num) }}

-   {% else %}

-     -

-   {% endif %}

+   {{ build_state_text(module.status | module_state_from_num) }}

  {% endmacro %}

  

  {% macro chroot_to_os_logo(chroot) %}

@@ -1,4 +1,4 @@ 

- [{{ copr.repo_id }}]

- name = Copr modules repo for {{ copr.full_name }}

- url = {{ copr.modules_url | fix_url_https_backend }}

+ [{{ copr.repo_id }}_{{ module.nsv }}]

+ name = Copr modules repo for {{ module.full_name }}

+ baseurl = {{ baseurl | fix_url_https_backend }}

  enabled = 1

@@ -37,30 +37,6 @@ 

  

        <div class="row">

  

- 

-         <div class="col-sm-12">

-           <div class="panel panel-default">

-             <div class="panel-body">

-               <div class="row">

-                 <div class="form-group col-sm-4">

-                   <label for="moduleName">Name:</label>

-                   <input type="text" class="form-control" id="moduleName" placeholder="{{ copr.name }}" disabled>

-                   <input type="hidden" value="{{ copr.name }}" name="name">

-                 </div>

-                 <div class="form-group col-sm-4">

-                   <label for="moduleRelease">Stream:</label>

-                   {{ render_field(form.stream, id="moduleStream") }}

-                 </div>

-                 <div class="form-group col-sm-4">

-                   <label for="moduleVersion">Version:</label>

-                   {{ render_field(form.version, id="moduleVersion") }}

-                 </div>

-               </div>

-             </div>

-           </div>

-         </div>

- 

- 

          {% for package, build in built_packages %}

            <input type="hidden" name="{{ 'packages-%s' |format(loop.index0) }}" value="{{ package }}">

            <input type="hidden" name="{{ 'builds-%s' |format(loop.index0) }}" value="{{ build.id }}">

@@ -110,6 +110,14 @@ 

                   -

              {% endif %}

            </dd>

+           {% if build.module %}

+           <dt> Module:</dt>

+           <dd>

+             <a href="{{ copr_url('coprs_ns.copr_module', copr, id=build.module.id) }}">

+               {{ build.module.nsv }}

+             </a>

+           </dd>

+           {% endif %}

            <dt> Version:</dt>

            <dd>

              {% if build.pkg_version %}

@@ -2,6 +2,7 @@ 

  {% from "coprs/detail/_builds_forms.html" import copr_build_cancel_form, copr_build_repeat_form, copr_build_delete_form %}

  {% from "coprs/detail/_describe_source.html" import describe_source %}

  {% from "coprs/detail/_describe_failure.html" import describe_failure %}

+ {% from "coprs/detail/_package_table.html" import package_table with context %}

  {% from "_helpers.html" import chroot_to_os_logo, build_state_text, build_state, copr_name %}

  {% block title %}Module {{ module.id }} in {{ copr_name(copr) }}{% endblock %}

  
@@ -148,6 +149,13 @@ 

          </div>

  

          <p>For <code>dnf module</code> documentation, see the "Module Command" section in <code>man dnf</code>.

+ 

+         <hr>

+         Repofiles:

+         {% for chroot in unique_chroots  %}

+           <a href="{{ copr_url('coprs_ns.generate_module_repo_file', copr, name_release=chroot.name_release, module_nsv=module.nsv) }}" class="btn btn-default"><span class="pficon pficon-save"></span> {{ chroot.name_release }}</a>

+         {% endfor %}

+ 

          {% endif %}

        </div>

      </div>
@@ -165,8 +173,9 @@ 

          {{ yaml | safe }}

        </div>

      </div>

-   </div>

  

+     {{package_table(module.builds)}}

+   </div>

  

  </div>

  

@@ -22,7 +22,7 @@ 

  from coprs.logic.complex_logic import ComplexLogic

  from coprs.logic.users_logic import UsersLogic

  from coprs.logic.packages_logic import PackagesLogic

- from coprs.logic.modules_logic import ModulesLogic

+ from coprs.logic.modules_logic import ModulesLogic, ModuleProvider, ModuleBuildFacade

  

  from coprs.views.misc import login_required, api_login_required

  
@@ -990,79 +990,35 @@ 

      })

  

  

- @api_ns.route("/module/build/", methods=["POST"])

- @api_login_required

- def copr_build_module():

-     form = forms.ModuleBuildForm(csrf_enabled=False)

-     if not form.validate_on_submit():

-         raise LegacyApiError(form.errors)

- 

-     try:

-         common = {"owner": flask.g.user.name,

-                   "copr_owner": form.copr_owner.data,

-                   "copr_project": form.copr_project.data}

-         if form.scmurl.data:

-             kwargs = {"json": dict({"scmurl": form.scmurl.data, "branch": form.branch.data}, **common)}

-         else:

-             kwargs = {"data": common, "files": {"yaml": (form.modulemd.data.filename, form.modulemd.data)}}

- 

-         response = requests.post(flask.current_app.config["MBS_URL"], verify=False, **kwargs)

-         if response.status_code == 500:

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

- 

-         resp = json.loads(response.content)

-         if response.status_code != 201:

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

- 

-         return flask.jsonify({

-             "output": "ok",

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

-         })

- 

-     except requests.ConnectionError:

-         raise LegacyApiError("Can't connect to MBS instance")

- 

- 

- @api_ns.route("/coprs/<username>/<coprname>/module/make/", methods=["POST"])

+ @api_ns.route("/coprs/<username>/<coprname>/module/build/", methods=["POST"])

  @api_login_required

  @api_req_with_copr

- def copr_make_module(copr):

-     form = forms.ModuleFormUploadFactory(csrf_enabled=False)

+ def copr_build_module(copr):

+     form = forms.ModuleBuildForm(csrf_enabled=False)

      if not form.validate_on_submit():

-         # @TODO Prettier error

          raise LegacyApiError(form.errors)

  

-     modulemd = form.modulemd.data.read()

-     module = ModulesLogic.from_modulemd(modulemd)

+     facade = None

      try:

-         ModulesLogic.validate(modulemd)

-         msg = "Nothing happened"

-         if form.create.data:

-             module = ModulesLogic.add(flask.g.user, copr, module)

-             db.session.flush()

-             msg = "Module was created"

- 

-         if form.build.data:

-             if not module.id:

-                 module = ModulesLogic.get_by_nsv(copr, module.name, module.stream, module.version).one()

-             ActionsLogic.send_build_module(flask.g.user, copr, module)

-             msg = "Module build was submitted"

+         mod_info = ModuleProvider.from_input(form.modulemd.data or form.scmurl.data)

+         facade = ModuleBuildFacade(flask.g.user, copr, mod_info.yaml, mod_info.filename)

+         module = facade.submit_build()

          db.session.commit()

  

          return flask.jsonify({

              "output": "ok",

-             "message": msg,

-             "modulemd": modulemd.decode("utf-8"),

+             "message": "Created module {}".format(module.nsv),

          })

  

-     except sqlalchemy.exc.IntegrityError:

-         raise LegacyApiError({"nsv": ["Module {} already exists".format(module.nsv)]})

+     except ValidationError as ex:

+         raise LegacyApiError(ex.message)

  

-     except sqlalchemy.orm.exc.NoResultFound:

-         raise LegacyApiError({"nsv": ["Module {} doesn't exist. You need to create it first".format(module.nsv)]})

+     except requests.RequestException as ex:

+         raise LegacyApiError(ex.message)

  

-     except ValidationError as ex:

-         raise LegacyApiError({"nsv": [ex.message]})

+     except sqlalchemy.exc.IntegrityError:

+         raise LegacyApiError("Module {}-{}-{} already exists".format(

+             facade.modulemd.name, facade.modulemd.stream, facade.modulemd.version))

  

  

  @api_ns.route("/coprs/<username>/<coprname>/build-config/<chroot>/", methods=["GET"])
@@ -1081,21 +1037,3 @@ 

          raise LegacyApiError('Chroot not found.')

  

      return flask.jsonify(output)

- 

- 

- @api_ns.route("/module/repo/", methods=["POST"])

- def copr_module_repo():

-     """

-     :return: URL to a DNF repository for the module

-     """

-     form = forms.ModuleRepo(csrf_enabled=False)

-     if not form.validate_on_submit():

-         raise LegacyApiError(form.errors)

- 

-     copr = ComplexLogic.get_copr_by_owner_safe(form.owner.data, form.copr.data)

-     nvs = [form.name.data, form.stream.data, form.version.data]

-     module = ModulesLogic.get_by_nsv(copr, *nvs).first()

-     if not module:

-         raise LegacyApiError("No module {}".format("-".join(nvs)))

- 

-     return flask.jsonify({"output": "ok", "repo": module.repo_url(form.arch.data)})

@@ -16,6 +16,7 @@ 

  import modulemd

  from email.mime.text import MIMEText

  from itertools import groupby

+ from wtforms import ValidationError

  

  from pygments import highlight

  from pygments.lexers import get_lexer_by_name
@@ -33,7 +34,7 @@ 

  from coprs.logic.packages_logic import PackagesLogic

  from coprs.logic.stat_logic import CounterStatLogic

  from coprs.logic.users_logic import UsersLogic

- from coprs.logic.modules_logic import ModulesLogic, ModulemdGenerator, MBSProxy

+ from coprs.logic.modules_logic import ModulesLogic, ModulemdGenerator, ModuleBuildFacade

  from coprs.rmodels import TimedStatEvents

  

  from coprs.logic.complex_logic import ComplexLogic
@@ -819,19 +820,23 @@ 

  ###                Module repo files                  ###

  #########################################################

  

- @coprs_ns.route("/<username>/<coprname>/repo/modules/")

- @coprs_ns.route("/@<group_name>/<coprname>/repo/modules/")

- @coprs_ns.route("/g/<group_name>/<coprname>/repo/modules/")

+ @coprs_ns.route("/<username>/<coprname>/module_repo/<name_release>/<module_nsv>")

+ @coprs_ns.route("/g/<group_name>/<coprname>/module_repo/<name_release>/<module_nsv>")

  @req_with_copr

- def generate_module_repo_file(copr):

+ def generate_module_repo_file(copr, name_release, module_nsv):

      """ Generate module repo file for a given project. """

-     return render_generate_module_repo_file(copr)

+     return render_generate_module_repo_file(copr, name_release, module_nsv)

  

- def render_generate_module_repo_file(copr):

+ def render_generate_module_repo_file(copr, name_release, module_nsv):

+     module = ModulesLogic.get_by_nsv_str(copr, module_nsv).one()

+     mock_chroot = coprs_logic.MockChrootsLogic.get_from_name(name_release, noarch=True).first()

      url = os.path.join(copr.repo_url, '') # adds trailing slash

+     repo_url = generate_repo_url(mock_chroot, copr.modules_url)

+     baseurl = "{}+{}/latest/$basearch".format(repo_url.rstrip("/"), module_nsv)

      pubkey_url = urljoin(url, "pubkey.gpg")

      response = flask.make_response(

-         flask.render_template("coprs/copr-modules.cfg", copr=copr, url=url, pubkey_url=pubkey_url))

+         flask.render_template("coprs/copr-modules.cfg", copr=copr, module=module,

+                               baseurl=baseurl, pubkey_url=pubkey_url))

      response.mimetype = "text/plain"

      response.headers["Content-Disposition"] = \

          "filename={0}.cfg".format(copr.repo_name)
@@ -991,26 +996,32 @@ 

          return render_create_module(copr, form, profiles=len(form.profile_names))

  

      summary = "Module from Copr repository: {}".format(copr.full_name)

-     generator = ModulemdGenerator(str(copr.name), str(form.stream.data),

-                                   form.version.data, summary, app.config)

+     generator = ModulemdGenerator(str(copr.name), summary=summary, config=app.config)

      generator.add_filter(form.filter.data)

      generator.add_api(form.api.data)

      generator.add_profiles(enumerate(zip(form.profile_names.data, form.profile_pkgs.data)))

      generator.add_components(form.packages.data, form.filter.data, form.builds.data)

-     generator.add_base_runtime()

-     tmp = tempfile.mktemp()

-     generator.dump(tmp)

+     yaml = generator.generate()

+ 

+     facade = None

+     try:

+         facade = ModuleBuildFacade(flask.g.user, copr, yaml)

+         module = facade.submit_build()

+         db.session.commit()

+ 

+         flask.flash("Modulemd yaml file successfully generated and submitted to be build as {}"

+                     .format(module.nsv), "success")

+         return flask.redirect(url_for_copr_details(copr))

  

-     proxy = MBSProxy(mbs_url=flask.current_app.config["MBS_URL"], user_name=flask.g.user.name)

-     with open(tmp) as tmp_handle:

-         response = proxy.build_module(copr.owner_name, copr.name, generator.nsv, tmp_handle)

-     os.remove(tmp)

+     except ValidationError as ex:

+         flask.flash(ex.message, "error")

+         return render_create_module(copr, form, len(form.profile_names))

  

-     if response.failed:

-         flask.flash(response.message, "error")

+     except sqlalchemy.exc.IntegrityError:

+         flask.flash("Module {}-{}-{} already exists".format(

+             facade.modulemd.name, facade.modulemd.stream, facade.modulemd.version), "error")

+         db.session.rollback()

          return render_create_module(copr, form, len(form.profile_names))

-     flask.flash("Modulemd yaml file successfully generated and submitted to be build", "success")

-     return flask.redirect(url_for_copr_details(copr))

  

  

  @coprs_ns.route("/<username>/<coprname>/module/<id>")
@@ -1020,7 +1031,20 @@ 

      module = ModulesLogic.get(id).first()

      formatter = HtmlFormatter(style="autumn", linenos=False, noclasses=True)

      pretty_yaml = highlight(module.yaml, get_lexer_by_name("YAML"), formatter)

-     return flask.render_template("coprs/detail/module.html", copr=copr, module=module, yaml=pretty_yaml)

+ 

+     # Get the list of chroots with unique name_release attribute

+     # Once we use jinja in 2.10 version, we can simply use

+     # {{ copr.active_chroots |unique(attribute='name_release') }}

+     unique_chroots = []

+     unique_name_releases = set()

+     for chroot in copr.active_chroots_sorted:

+         if chroot.name_release in unique_name_releases:

+             continue

+         unique_chroots.append(chroot)

+         unique_name_releases.add(chroot.name_release)

+ 

+     return flask.render_template("coprs/detail/module.html", copr=copr, module=module,

+                                  yaml=pretty_yaml, unique_chroots=unique_chroots)

  

  

  @coprs_ns.route("/<username>/<coprname>/module/<id>/raw")

@@ -38,8 +38,9 @@ 

  

      def test_generate_repo_url(self):

          test_sets = []

-         http_url = "http://example.com/repo"

-         https_url = "https://example.com/repo"

+ 

+         http_url = "http://example.com/path"

+         https_url = "https://example.com/path"

  

          mock_chroot = mock.MagicMock()

          mock_chroot.os_release = "fedora"
@@ -47,18 +48,18 @@ 

  

          test_sets.extend([

              dict(args=(mock_chroot, http_url),

-                  expected="http://example.com/fedora-$releasever-$basearch/"),

+                  expected="http://example.com/path/fedora-$releasever-$basearch/"),

              dict(args=(mock_chroot, https_url),

-                  expected="https://example.com/fedora-$releasever-$basearch/")])

+                  expected="https://example.com/path/fedora-$releasever-$basearch/")])

  

          m2 = deepcopy(mock_chroot)

          m2.os_version = "rawhide"

  

          test_sets.extend([

              dict(args=(m2, http_url),

-                  expected="http://example.com/fedora-rawhide-$basearch/"),

+                  expected="http://example.com/path/fedora-rawhide-$basearch/"),

              dict(args=(m2, https_url),

-                  expected="https://example.com/fedora-rawhide-$basearch/")])

+                  expected="https://example.com/path/fedora-rawhide-$basearch/")])

  

          m3 = deepcopy(mock_chroot)

          m3.os_release = "rhel7"
@@ -66,9 +67,9 @@ 

  

          test_sets.extend([

              dict(args=(m3, http_url),

-                  expected="http://example.com/rhel7-7.1-$basearch/"),

+                  expected="http://example.com/path/rhel7-7.1-$basearch/"),

              dict(args=(m3, https_url),

-                  expected="https://example.com/rhel7-7.1-$basearch/")])

+                  expected="https://example.com/path/rhel7-7.1-$basearch/")])

  

          app.config["USE_HTTPS_FOR_RESULTS"] = True

          for test_set in test_sets:

@@ -103,62 +103,3 @@ 

      #     self.db.session.add_all([self.u1, self.mc1])

      #

      #

- 

- 

- class TestModuleRepo(CoprsTestCase):

-     endpoint = "/api/module/repo/"

- 

-     def test_api_module_repo(self, f_users, f_coprs, f_modules, f_db):

-         data = {"owner": self.u1.name, "copr": self.c1.name, "name": "first-module",

-                 "stream": "foo", "version": 1, "arch": "x86_64"}

- 

-         r = self.tc.post(self.endpoint, data=data)

-         response = json.loads(r.data.decode("utf-8"))

-         assert response["output"] == "ok"

-         assert response["repo"] == "http://copr-be-dev.cloud.fedoraproject.org/results/user1/foocopr/modules/"\

-                                    "fedora-24-x86_64+first-module-foo-1/latest/x86_64"

- 

-     def test_api_module_repo_no_params(self):

-         error = "This field is required."

-         r = self.tc.post(self.endpoint, data={})

-         response = json.loads(r.data.decode("utf-8"))

-         assert response["output"] == "notok"

-         for key in ["owner", "copr", "name", "stream", "version", "arch"]:

-             assert error in response["error"][key]

- 

- 

- class TestBuildModule(CoprsTestCase):

-     @TransactionDecorator("u1")

-     def test_api_build_module_basic(self, f_users, f_coprs, f_db):

-         self.db.session.add_all([self.u1, self.c1])

-         self.tc.post("/api/new/")

- 

-         fd, filename = tempfile.mkstemp()

-         os.write(fd, b"""

-         data:

-           api:

-             rpms: [example-debuginfo]

-           components: {}

-           description: ''

-           filter:

-             rpms: [example-debuginfo, example]

-           license:

-             module: []

-           name: project

-           profiles:

-             default:

-               rpms: [example]

-           stream: test

-           summary: 'Module from Copr repository: clime/project'

-           version: 1

-         document: modulemd

-         version: 1

-         """)

-         os.close(fd)

- 

-         f = open(filename, "rb")

-         data = {"modulemd": (filename, f.read().decode("utf-8"), "application/yaml")}

-         api_endpoint = '/api/coprs/{}/{}/module/make/'.format(self.u1.name, self.c1.name)

-         r = self.post_api_with_auth(api_endpoint, content=data, user=self.u1)

-         assert r.status_code == 200

-         os.remove(filename)

file modified
+8 -29
@@ -1599,14 +1599,17 @@ 

          return response

  

      def build_module(self, modulemd, ownername=None, projectname=None):

-         endpoint = "module/build"

-         url = "{}/{}/".format(self.api_url, endpoint)

+         if not ownername:

+             ownername = self.username

+ 

+         url = "{0}/coprs/{1}/{2}/module/build/".format(

+             self.api_url, ownername, projectname

+         )

  

-         data = {"copr_owner": ownername, "copr_project": projectname}

          if isinstance(modulemd, io.BufferedIOBase):

-             data.update({"modulemd": (os.path.basename(modulemd.name), modulemd, "application/x-rpm")})

+             data = {"modulemd": (os.path.basename(modulemd.name), modulemd, "application/x-rpm")}

          else:

-             data.update({"scmurl": modulemd, "branch": "master"})

+             data = {"scmurl": modulemd, "branch": "master"}

  

          def fetch(url, data, method):

              m = MultipartEncoder(data)
@@ -1616,27 +1619,3 @@ 

          # @TODO Refactor process_package_action to be general purpose

          response = self.process_package_action(url, None, None, data=data, fetch_functor=fetch)

          return response

- 

-     def make_module(self, projectname, modulemd, username=None, create=True, build=True):

-         api_endpoint = "module/make"

-         ownername = username if username else self.username

-         f = open(modulemd, "rb")

-         data = {

-             "modulemd": (os.path.basename(f.name), f, "application/x-rpm"),

-             "username": self.username,

-             "create": "y" if create else "",

-             "build": "y" if build else "",

-         }

- 

-         url = "{0}/coprs/{1}/{2}/{3}/".format(

-             self.api_url, ownername, projectname, api_endpoint

-         )

- 

-         def fetch(url, data, method):

-             m = MultipartEncoder(data)

-             monit = MultipartEncoderMonitor(m, lambda x: x)

-             return self._fetch(url, monit, method="post", headers={'Content-Type': monit.content_type})

- 

-         # @TODO Refactor process_package_action to be general general purpose

-         response = self.process_package_action(url, ownername, projectname, data=data, fetch_functor=fetch)

-         return response

This PR is based on recent article https://communityblog.fedoraproject.org/modularity-dead-long-live-modularity/ . Please read it to get all available information. Though, I will note the relevant information here and describe how changes in the code relate to it.

Article conclusion

It was decided that the current Modularity design is going to be changed. And by changed I rather mean simplified. Copr is mostly affected by following:

  • There is not going to be a buildroot composed of modules. Instead, the standard Fedora buildroot will be used
  • "Fedora will ship with two sets of repositories. One will be the traditional Fedora repositories (fedora, updates, and updates-testing) and the other will be a new set of repositories providing alternative and supplementary modules."

Features

  • Build module packages in standard Fedora buildroot instead of the custom-1-x86_64. This comes directly from the article. We will now build module packages in all enabled chroots of the project.
  • Do not use MBS to build modules. There are numbers of reasons for this and I want to describe them in depth, so there is dedicated full paragraph for this. Please read further.
  • Have the unique module NSV per project. IIRC, it was unique just per user before.
  • The web UI for creating module was temporarily disabled since we don't yet know what input fields there should be.
  • When you open a module detail page, you can see the list of (package) builds which was created on behalf of the particular module. Similarly, when you open a build detail page, there is shown a module that the build is related to. Ofc, if it is a regular build and is not part of a module, nothing is shown.
  • Bugfix: Always know module state; We had an issue, that the module state could be unknown, so the "-" was displayed instead

CLI changes

  • A user always needs to specify the project in which module should be built. Before, the project parameter could be omitted and in such case, a new project was created and configured. We don't need any complex project configuration anymore, so we can reduce the confusion of implicit project creation.
  • The --url parameter doesn't expect a confusing (RhBug: 1515813) SCM URL with git ref in it, but rather a link to a YAML file.

Follow up

As usual, there are a lot of not yet known things, such as

  • What is the fate of module-build-macros? I assume that they will be killed, so they are not used in this PR
  • There will be official modular repos that could be enabled on a regular fedora system. Should they be allowed in the buildroot?
  • The workflow for creating a repodata on backend was not changed. After all the packages from the module are built, an Action is sent to create a repodata for the module. I think that there is a room for improvement and no action is going to be required.
  • At least beaker tests should be done
  • ...

Some of this things are not known yet, some of them could be done even now. However this PR is huge just as is, and I don't want to add more features by now. I consider this state as a minimal working implementation and I would like to just improve it and also write the tests and fix the bugs. Features should come after proper planning.

Dropping MBS

Here comes the biggest change for us and nearly invisible change for users - I believe that we don't need the MBS anymore. When we started implementing modularity features in the Copr more than a year ago, a lot of things was not known. Moreover, it wasn't on us to figure them out. At that time, using an MBS was a great idea for various reasons. It provided us a nice black box for all the modularity magic which was constantly reworked and allowed us to easily implement a builder class, which communicated with copr frontend. At this point, MBS does following non-trivial things for us:

  1. Communicating with PDC to resolve module dependencies
  2. Creating a module-build-macros package that alters disttag and also defines user (defined in modulemd yaml) macros in the buildroot
  3. Scheduling package builds and managing the buildorder
  4. Nothing more actually.

Now, when the design is greatly simplified, we IMHO don't have to resolve any dependencies. We have standard buildroot, and DNF should handle the dependencies by itself. Regarding the module-build-macros, I suppose that it will be killed. We can't be sure until someone confirms it, but we can live without them for now. We are able to manage batches by ourselves because @clime recently implemented a Batch feature.

Well, this was an explanation why we can drop the MBS, now let's talk about why we want to drop the MBS.

  1. We will not have to maintain the MBS instance
  2. The communication between MBS and frontend is done via API and requires proxy user. We can get rid of that. The communication between MBS and backend is done via fedmsg. All of this creates a lot of opportunities for bugs and it is definitely not fun to debug.
  3. Pieces of information are in several places. We would like to have everything in our database, but MBS manages its own database. We don't actually depend on it, but we need to consider this database when chasing bugs.
  4. The codebase is not that big, but it is another code that we need to know.

The whole time I was defending the MBS and I am convinced that it was a good idea to use it. However, I think that we don't need it anymore and it brings us more time-consuming complications than benefits.

Example usage

# Build
copr-cli build-module --url https://raw.githubusercontent.com/asamalik/testmodule/master/testmodule.yaml frostyx/foo

# Enable repo
wget http://127.0.0.1:5000/coprs/frostyx/foo/module_repo/fedora-rawhide/testmodule-master-20171230202128 -O /etc/yum.repos.d/coprmodule.repo

# Work with it
dnf module list

Here are some screenshots, so you can easily see how the UI looks like

I think further simplifications might follow but at this point LGTM.

Cool. If you have any particular simplifications in mind, please let me know, so we don't forget any of them.

Also, thinking of Mirek's horrified face when I said, that the webform is temporarily disabled, I would like to finish it in this PR. Which is actually not bad, because it will allow us to remove more of the outdated code.

Cool. If you have any particular simplifications in mind, please let me know, so we don't forget any of them.

We will see. I think the current state is ok considering what has been written in the blog post. We can make improvements on the way.

I wouldn't necessarily react to Mirek's horrified face on UI thing. Maybe we could get some good user feedback when the UI was completely disabled for a bit (i.e. we could get information what users actually need to have there but that's just a wild theory :)).

5 new commits added

  • [frontend][backend] copy only module builds into the repo directory
  • [frontend] generate the module NSV rather than asking for it
  • [frontend] fix condition that all module packages were successfully built
  • [frontend] re-enable webform for building modules
  • [frontend] show module repofiles
6 years ago

Can you please review these last commits, @clime?

I've re-enabled the web UI for creating modules and also showed repofiles on the module detail page. There are also two bugfixes. There was a mistake in module status logic, which made it successful even though some packages were still running and also I found a bug in action handler on backend. It duplicated all packages built in the particular chroot into the module directory. I modified it, to just duplicate packages that were built on behalf of the module.

Well, the bugfixes are certainly ok. For the rest...I am unclear about the purpose so I'll just give you completely free hand in all this.

This columnt should have index. Foreign key is integrity constraint, it is not used for speed up queries.

Generally, it is better to rewrite it to exists() rather than all() as it is faster. E.g., ... and not exists(b.status != StatusEnum("succeeded") for b in build.module.builds)

2 new commits added

  • [frontend] add index to build module_id
  • [frontend] use exists instead of all, it is faster
6 years ago

3 new commits added

  • [python] use username from config if nothing is explicitly specified
  • [frontend] when passing URL with path, expect it in result; see ad9c3b4cc
  • [frontend] remove outdated tests, see 3f62873
6 years ago

1 new commit added

  • Revert "[frontend] use exists instead of all, it is faster"
6 years ago

I needed to revert the b66fef1 because it caused the "Boolean value of this clause is not defined" error.

Also, please see the PR#195 for beaker tests.

Since the tests are passing, I think that this PR is good to go.

Yes, good by me.

We really need those tests to be run automatically.

rebased onto 041d651

6 years ago

There was a merge conflict, so I've rebased the branch and fixed two issues caused by the recent switch to python3. Tests are passing, so I am going to merge it.

Pull-Request has been merged by frostyx

6 years ago
Metadata
Changes Summary 22
+10 -2
file changed
backend/backend/actions.py
+2 -2
file changed
cli/copr_cli/main.py
+22
file added
frontend/coprs_frontend/alembic/schema/versions/1f94b22f70a1_change_module_version_to_bigint.py
+24
file added
frontend/coprs_frontend/alembic/schema/versions/3b0851cb25fc_add_build_to_module_relation.py
+22
file added
frontend/coprs_frontend/alembic/schema/versions/e183e12563ee_add_index_to_module_id.py
+22
file added
frontend/coprs_frontend/alembic/schema/versions/f61a5c930abf_unique_constraint_on_modules.py
+0 -11
file changed
frontend/coprs_frontend/coprs/forms.py
+2 -1
file changed
frontend/coprs_frontend/coprs/helpers.py
+2 -4
file changed
frontend/coprs_frontend/coprs/logic/actions_logic.py
+7 -0
file changed
frontend/coprs_frontend/coprs/logic/builds_logic.py
+87 -10
file changed
frontend/coprs_frontend/coprs/logic/modules_logic.py
+18 -11
file changed
frontend/coprs_frontend/coprs/models.py
+1 -5
file changed
frontend/coprs_frontend/coprs/templates/_helpers.html
+3 -3
file changed
frontend/coprs_frontend/coprs/templates/coprs/copr-modules.cfg
+0 -24
file changed
frontend/coprs_frontend/coprs/templates/coprs/create_module.html
+8 -0
file changed
frontend/coprs_frontend/coprs/templates/coprs/detail/build.html
+10 -1
file changed
frontend/coprs_frontend/coprs/templates/coprs/detail/module.html
+16 -78
file changed
frontend/coprs_frontend/coprs/views/api_ns/api_general.py
+46 -22
file changed
frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+9 -8
file changed
frontend/coprs_frontend/tests/test_helpers.py
+0 -59
file changed
frontend/coprs_frontend/tests/test_views/test_api_ns/test_api_general.py
+8 -29
file changed
python/copr/client/client.py