PR#734: [frontend][backend] remove unused module_md_name and module_md_zlib columns - copr/copr

copr / copr

#734 [frontend][backend] remove unused module_md_name and module_md_zlib columns

Merged 4 years ago by frostyx. Opened 4 years ago by frostyx.

copr/ frostyx/copr modulemd-yaml-error-quark into master

[frontend][backend] remove unused module_md_name and module_md_zlib columns

Jakub Kadlčík • 4 years ago

da34d33

backend/backend/actions.py

file modified

+1 -33

		`@@ -220,35 +220,6 @@`
		`remote_comps_url, local_comps_path)`
		`result.result = ActionResult.FAILURE`

		`- def handle_module_md_update(self, result):`
		`- self.log.debug("Action module_md update")`
		`-`
		`- ext_data = json.loads(self.data["data"])`
		`- ownername = ext_data["ownername"]`
		`- projectname = ext_data["projectname"]`
		`- chroot = ext_data["chroot"]`
		`- url_path = ext_data["url_path"]`
		`-`
		`- remote_module_md_url = self.opts.frontend_base_url + url_path`
		`-`
		`- path = self.get_chroot_result_dir(chroot, projectname, ownername)`
		`- ensure_dir_exists(path, self.log)`
		`- local_module_md_path = os.path.join(path, "module_md.yaml")`
		`- result.result = ActionResult.SUCCESS`
		`- if not ext_data.get("module_md_present", True):`
		`- silent_remove(local_module_md_path)`
		`- self.log.info("deleted module_md.yaml for %s/%s/%s from %s ",`
		`- ownername, projectname, chroot, remote_module_md_url)`
		`- else:`
		`- try:`
		`- urlretrieve(remote_module_md_url, local_module_md_path)`
		`- self.log.info("saved module_md.yaml for %s/%s/%s from %s ",`
		`- ownername, projectname, chroot, remote_module_md_url)`
		`- except Exception:`
		`- self.log.exception("Failed to update module_md from %s at location %s",`
		`- remote_module_md_url, local_module_md_path)`
		`- result.result = ActionResult.FAILURE`
		`-`
		`def handle_delete_build(self):`
		`self.log.info("Action delete build.")`

		`@@ -498,9 +469,6 @@`
		`elif action_type == ActionType.RAWHIDE_TO_RELEASE:`
		`self.handle_rawhide_to_release(result)`

		`- elif action_type == ActionType.UPDATE_MODULE_MD:`
		`- self.handle_module_md_update(result)`
		`-`
		`elif action_type == ActionType.BUILD_MODULE:`
		`self.handle_build_module(result)`

		`@@ -530,7 +498,7 @@`
		`GEN_GPG_KEY = 5`
		`RAWHIDE_TO_RELEASE = 6`
		`FORK = 7`
		`- UPDATE_MODULE_MD = 8`
		`+ UPDATE_MODULE_MD = 8 # Deprecated`
		`BUILD_MODULE = 9`
		`CANCEL_BUILD = 10`

backend/backend/mockremote/builder.py

file modified

+1 -16

		`@@ -43,24 +43,9 @@`
		`config_file=self.opts.ssh.builder_config`
		`)`

		`- self.module_dist_tag = self._load_module_dist_tag()`
		`+ self.module_dist_tag = None`
		`self._build_pid = None`

		`- def _load_module_dist_tag(self):`
		`- module_md_filepath = os.path.join(self.job.destdir, self.job.chroot, "module_md.yaml")`
		`- try:`
		`- 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:`
		`- self.log.exception(str(e))`
		`- return None`
		`- else:`
		`- self.log.info("Loaded {}, dist_tag {}".format(module_md_filepath, dist_tag))`
		`- return dist_tag`
		`-`
		`def _run_ssh_cmd(self, cmd):`
		`"""`
		`Executes single shell command remotely`

frontend/coprs_frontend/alembic/schema/versions/2d8b4722918b_remove_unused_module_md_name_and_module_.py

file added

+22

		`@@ -0,0 +1,22 @@`
		`+ """`
		`+ Remove unused module_md_name and module_md_zlib columns`
		`+`
		`+ Revision ID: 2d8b4722918b`
		`+ Revises: 8ae65946df53`
		`+ Create Date: 2019-05-05 17:42:00.875012`
		`+ """`
		`+`
		`+ import sqlalchemy as sa`
		`+ from alembic import op`
		`+ from sqlalchemy.dialects import postgresql`
		`+`
		`+ revision = '2d8b4722918b'`
		`+ down_revision = '8ae65946df53'`
		`+`
		`+ def upgrade():`
		`+ op.drop_column('copr_chroot', 'module_md_name')`
		`+ op.drop_column('copr_chroot', 'module_md_zlib')`
		`+`
		`+ def downgrade():`
		`+ op.add_column('copr_chroot', sa.Column('module_md_zlib', postgresql.BYTEA(), autoincrement=False, nullable=True))`
		`+ op.add_column('copr_chroot', sa.Column('module_md_name', sa.VARCHAR(length=127), autoincrement=False, nullable=True))`

frontend/coprs_frontend/coprs/forms.py

file modified

-2

		`@@ -956,8 +956,6 @@`
		`wtforms.validators.Optional()],`
		`filters=[StringListFilter()])`

		`- module_md = FileField("module_md")`
		`-`
		`comps = FileField("comps_xml")`

		`with_opts = wtforms.TextField("With options")`

frontend/coprs_frontend/coprs/logic/actions_logic.py

file modified

-23

		`@@ -170,29 +170,6 @@`
		`db.session.add(action)`

		`@classmethod`
		`- def send_update_module_md(cls, chroot):`
		`- """ Schedules update module_md.yaml action`
		`-`
		`- :type copr_chroot: models.CoprChroot`
		`- """`
		`- url_path = helpers.copr_url("coprs_ns.chroot_view_module_md", chroot.copr, chrootname=chroot.name)`
		`- data_dict = {`
		`- "ownername": chroot.copr.owner_name,`
		`- "projectname": chroot.copr.name,`
		`- "chroot": chroot.name,`
		`- "module_md_present": chroot.module_md_zlib is not None,`
		`- "url_path": url_path,`
		`- }`
		`-`
		`- action = models.Action(`
		`- action_type=ActionTypeEnum("update_module_md"),`
		`- object_type="copr_chroot",`
		`- data=json.dumps(data_dict),`
		`- created_on=int(time.time())`
		`- )`
		`- db.session.add(action)`
		`-`
		`- @classmethod`
		`def send_create_gpg_key(cls, copr):`
		`"""`
		`:type copr: models.Copr`

frontend/coprs_frontend/coprs/logic/coprs_logic.py

file modified

+5 -22

		`@@ -629,7 +629,7 @@`

		`@classmethod`
		`def create_chroot(cls, user, copr, mock_chroot, buildroot_pkgs=None, repos=None, comps=None, comps_name=None,`
		`- module_md=None, module_md_name=None, with_opts="", without_opts="",`
		`+ with_opts="", without_opts="",`
		`delete_after=None, delete_notify=None):`
		`"""`
		`:type user: models.User`
		`@@ -644,14 +644,13 @@`
		`"Only owners and admins may update their projects.")`

		`chroot = models.CoprChroot(copr=copr, mock_chroot=mock_chroot)`
		`- cls._update_chroot(buildroot_pkgs, repos, comps, comps_name, module_md, module_md_name, chroot,`
		`+ cls._update_chroot(buildroot_pkgs, repos, comps, comps_name, chroot,`
		`with_opts, without_opts, delete_after, delete_notify)`
		`return chroot`

		`@classmethod`
		`def update_chroot(cls, user, copr_chroot, buildroot_pkgs=None, repos=None, comps=None, comps_name=None,`
		`- module_md=None, module_md_name=None, with_opts="", without_opts="",`
		`- delete_after=None, delete_notify=None):`
		`+ with_opts="", without_opts="", delete_after=None, delete_notify=None):`
		`"""`
		`:type user: models.User`
		`:type copr_chroot: models.CoprChroot`
		`@@ -660,12 +659,12 @@`
		`user, copr_chroot.copr,`
		`"Only owners and admins may update their projects.")`

		`- cls._update_chroot(buildroot_pkgs, repos, comps, comps_name, module_md, module_md_name,`
		`+ cls._update_chroot(buildroot_pkgs, repos, comps, comps_name,`
		`copr_chroot, with_opts, without_opts, delete_after, delete_notify)`
		`return copr_chroot`

		`@classmethod`
		`- def _update_chroot(cls, buildroot_pkgs, repos, comps, comps_name, module_md, module_md_name,`
		`+ def _update_chroot(cls, buildroot_pkgs, repos, comps, comps_name,`
		`copr_chroot, with_opts, without_opts, delete_after, delete_notify):`
		`if buildroot_pkgs is not None:`
		`copr_chroot.buildroot_pkgs = buildroot_pkgs`
		`@@ -684,11 +683,6 @@`
		`copr_chroot.comps_name = comps_name`
		`ActionsLogic.send_update_comps(copr_chroot)`

		`- if module_md_name is not None:`
		`- copr_chroot.update_module_md(module_md)`
		`- copr_chroot.module_md_name = module_md_name`
		`- ActionsLogic.send_update_module_md(copr_chroot)`
		`-`
		`if delete_after is not None:`
		`copr_chroot.delete_after = delete_after`

		`@@ -736,17 +730,6 @@`
		`db.session.add(copr_chroot)`

		`@classmethod`
		`- def remove_module_md(cls, user, copr_chroot):`
		`- UsersLogic.raise_if_cant_update_copr(`
		`- user, copr_chroot.copr,`
		`- "Only owners and admins may update their projects.")`
		`-`
		`- copr_chroot.module_md_name = None`
		`- copr_chroot.module_md_zlib = None`
		`- ActionsLogic.send_update_module_md(copr_chroot)`
		`- db.session.add(copr_chroot)`
		`-`
		`- @classmethod`
		`def remove_copr_chroot(cls, user, copr_chroot):`
		`"""`
		`:param models.CoprChroot chroot:`

frontend/coprs_frontend/coprs/models.py

file modified

-22

		`@@ -1161,9 +1161,6 @@`
		`comps_zlib = db.Column(db.LargeBinary(), nullable=True)`
		`comps_name = db.Column(db.String(127), nullable=True)`

		`- module_md_zlib = db.Column(db.LargeBinary(), nullable=True)`
		`- module_md_name = db.Column(db.String(127), nullable=True)`
		`-`
		`with_opts = db.Column(db.Text, default="", server_default="", nullable=False)`
		`without_opts = db.Column(db.Text, default="", server_default="", nullable=False)`

		`@@ -1179,13 +1176,6 @@`
		`data = comps_xml`
		`self.comps_zlib = zlib.compress(data)`

		`- def update_module_md(self, module_md_yaml):`
		`- if isinstance(module_md_yaml, str):`
		`- data = module_md_yaml.encode("utf-8")`
		`- else:`
		`- data = module_md_yaml`
		`- self.module_md_zlib = zlib.compress(data)`
		`-`
		`@property`
		`def buildroot_pkgs_list(self):`
		`return (self.buildroot_pkgs or "").split()`
		`@@ -1200,11 +1190,6 @@`
		`return zlib.decompress(self.comps_zlib).decode("utf-8")`

		`@property`
		`- def module_md(self):`
		`- if self.module_md_zlib:`
		`- return zlib.decompress(self.module_md_zlib).decode("utf-8")`
		`-`
		`- @property`
		`def comps_len(self):`
		`if self.comps_zlib:`
		`return len(zlib.decompress(self.comps_zlib))`
		`@@ -1212,13 +1197,6 @@`
		`return 0`

		`@property`
		`- def module_md_len(self):`
		`- if self.module_md_zlib:`
		`- return len(zlib.decompress(self.module_md_zlib))`
		`- else:`
		`- return 0`
		`-`
		`- @property`
		`def name(self):`
		`return self.mock_chroot.name`

frontend/coprs_frontend/coprs/templates/coprs/detail/edit_chroot.html

file modified

-27

		`@@ -75,33 +75,6 @@`

		`</div>`

		`- <div class="form-group">`
		`- <label class="col-sm-2 control-label" for="textInput-markup">`
		`- module_md.yaml`
		`- </label>`
		`- <div class="col-sm-10">`
		`- {{ form.module_md }}`
		`- {% if chroot.module_md_name %}`
		`- Stored module meta-data file: <code>`
		`- <a href="{{ copr_url('coprs_ns.chroot_view_module_md', copr, chrootname=chroot.name) }}">`
		`- {{ chroot.module_md_name }}`
		`- </a>`
		`-`
		`- </code>`
		`- <button class="btn btn-default" type="submit" value="delete_module_md" name="submit">`
		`- <span class="pficon pficon-delete"></span> Delete`
		`- </button>`
		`- {% endif %}`
		`- <li class="help-block">`
		`- <small><span class="text-muted pficon pficon-info"></span>`
		`- If you would like to provide module meta-data, please upload your module_md.yaml here`
		`- </small>`
		`- </li>`
		`-`
		`- </div>`
		`-`
		`- </div>`
praiskup commented 4 years ago Only this project seem to me to be ever using this form: https://copr.fedorainfracloud.org/coprs/cottsay/gringo/
		`-`
		`<input class="btn btn-primary" type="submit" name="submit" value="update">`
		`<a class="btn btn-default" href="{{ copr_url('coprs_ns.copr_edit', copr) }}">Cancel</a>`
		`</form>`

frontend/coprs_frontend/coprs/views/coprs_ns/coprs_chroots.py

file modified

-21

		`@@ -63,31 +63,22 @@`
		`action = flask.request.form["submit"]`
		`if action == "update":`
		`comps_name = comps_xml = None`
		`- module_md_name = module_md = None`

		`if form.comps.has_file():`
		`comps_xml = form.comps.data.stream.read()`
		`comps_name = form.comps.data.filename`

		`- if form.module_md.has_file():`
		`- module_md = form.module_md.data.stream.read()`
		`- module_md_name = form.module_md.data.filename`
		`-`
		`coprs_logic.CoprChrootsLogic.update_chroot(`
		`flask.g.user, chroot,`
		`form.buildroot_pkgs.data,`
		`form.repos.data,`
		`comps=comps_xml, comps_name=comps_name,`
		`- module_md=module_md, module_md_name=module_md_name,`
		`with_opts=form.with_opts.data, without_opts=form.without_opts.data`
		`)`

		`elif action == "delete_comps":`
		`CoprChrootsLogic.remove_comps(flask.g.user, chroot)`

		`- elif action == "delete_module_md":`
		`- CoprChrootsLogic.remove_module_md(flask.g.user, chroot)`
		`-`
		`flask.flash(`
		`"Buildroot {0} in project {1} has been updated successfully.".format(`
		`chroot_name, copr.name), 'success')`
		`@@ -110,15 +101,3 @@`
		`def render_chroot_view_comps(copr, chroot_name):`
		`chroot = ComplexLogic.get_copr_chroot_safe(copr, chroot_name)`
		`return Response(chroot.comps or "", mimetype="text/plain; charset=utf-8")`
		`-`
		`-`
		`- @coprs_ns.route("/<username>/<coprname>/chroot/<chrootname>/module_md/")`
		`- @coprs_ns.route("/g/<group_name>/<coprname>/chroot/<chrootname>/module_md/")`
		`- @req_with_copr`
		`- def chroot_view_module_md(copr, chrootname):`
		`- return render_chroot_view_module_md(copr, chrootname)`
		`-`
		`-`
		`- def render_chroot_view_module_md(copr, chroot_name):`
		`- chroot = ComplexLogic.get_copr_chroot_safe(copr, chroot_name)`
		`- return Response(chroot.module_md or "", mimetype="text/plain; charset=utf-8")`

frostyx commented 4 years ago

Fix #707

Those two columns were added in d8ae899 as a very first prototype
of building modules in Copr, but not being used since then. Now, we
rather have a module table and store a modulemd yaml within it.