PR#958: Added search for modules by binary rpm. - fm-orchestrator

fm-orchestrator

#958 Added search for modules by binary rpm.

Merged 5 years ago by mcurlej. Opened 5 years ago by mcurlej.

mcurlej/fm-orchestrator binary-rpm into master

Added search for modules by binary rpm.

Martin Curlej • 5 years ago

0aa9f01

module_build_service/builder/KojiModuleBuilder.py

file modified

+23

		`@@ -1064,3 +1064,26 @@`
		`if self.config.koji_enable_content_generator and self.module.state == 3:`
		`cg = KojiContentGenerator(self.module, self.config)`
		`cg.koji_import()`
		`+`
		`+ @staticmethod`
		`+ def get_rpm_module_tag(rpm):`
		`+ """`
		`+ Returns koji tag of a given rpm filename.`
		`+`
		`+ :param str rpm: the *.rpm filename of a rpm`
		`+ :rtype: str`
		`+ :return: koji tag`
		`+ """`
		`+`
		`+ session = KojiModuleBuilder.get_session(conf, None)`
		`+ rpm_md = session.getRPM(rpm)`
		`+ if not rpm_md:`
		`+ return None`
		`+`
		`+ tags = []`
		`+ koji_tags = session.listTags(rpm_md["build_id"])`
		`+ for t in koji_tags:`
		`+ if not t["name"].endswith("-build") and t["name"].startswith("module-"):`
		`+ tags.append(t["name"])`
		`+`
		`+ return tags`

module_build_service/utils/views.py

file modified

+14 -1

		`@@ -30,7 +30,7 @@`
		`from flask import request, url_for, Response`
		`from sqlalchemy.sql.sqltypes import Boolean as sqlalchemy_boolean`

		`- from module_build_service import models, api_version`
		`+ from module_build_service import models, api_version, conf`
		`from module_build_service.errors import ValidationError, NotFound`
		`from .general import scm_url_schemes`

		`@@ -212,12 +212,25 @@`
		`for key, part in zip(query_keys, nsvc_parts):`
		`search_query[key] = part`

		`+ rpm = flask_request.args.get('rpm', None)`
		`+ koji_tags = []`
		`+ if rpm:`
		`+ if conf.system == "koji":`
		`+ # we are importing the koji builder here so we can search for the rpm metadata`
		`+ # from koji. If we imported this regulary we would have gotten a circular import error.`
		`+ from module_build_service.builder.KojiModuleBuilder import KojiModuleBuilder # noqa`
		`+ koji_tags = KojiModuleBuilder.get_rpm_module_tag(rpm)`
		`+ else:`
		`+ raise ValidationError("Configured builder does not allow to search by rpm binary name!")`
		`+`
		`query = models.ModuleBuild.query`

		`if search_query:`
		`query = query.filter_by(**search_query)`
		`if search_states:`
		`query = query.filter(models.ModuleBuild.state.in_(search_states))`
		`+ if koji_tags:`
		`+ query = query.filter(models.ModuleBuild.koji_tag.in_(koji_tags)).filter_by(**search_query)`

		`# This is used when filtering the date request parameters, but it is here to avoid recompiling`
		`utc_iso_datetime_regex = re.compile(`

tests/test_views/test_views.py

file modified

+48 -2

		`@@ -24,14 +24,15 @@`

		`import module_build_service.scm`

		`- from mock import patch, PropertyMock`
		`+ from mock import patch, PropertyMock, Mock`
		`from shutil import copyfile`
		`from os import path, mkdir`
		`from os.path import dirname`
		`+ from requests.utils import quote`
		`import hashlib`
		`import pytest`

		`- from tests import app, init_data, clean_database`
		`+ from tests import app, init_data, clean_database, reuse_component_init_data`
		`from module_build_service.errors import UnprocessableEntity`
		`from module_build_service.models import ModuleBuild`
		`from module_build_service import db, version, Modulemd`
		`@@ -392,6 +393,51 @@`
		`for key, part in zip(nsvc_keys, nsvc_parts):`
		`assert item[key] == part`

		`+ @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")`
		`+ def test_query_builds_with_binary_rpm(self, mock_get_session):`
		`+ """`
		`+ Test for querying MBS with the binary rpm filename. MBS should return all the modules,`
		`+ which contain the rpm.`
		`+ """`
		`+ # update database with builds which contain koji tags.`
		`+ reuse_component_init_data()`
		`+ mock_rpm_md = {"build_id": 1065871}`
		`+ mock_tags = [{"name": "module-testmodule-master-20170219191323-c40c156c"},`
		`+ {"name": "module-testmodule-master-20170219191323-c40c156c-build"},`
		`+ {"name": "non-module-tag"},`
		`+ {"name": "module-testmodule-master-20170109091357-78e4a6fd"}]`
		`+`
		`+ mock_session = Mock()`
		`+ mock_session.getRPM.return_value = mock_rpm_md`
		`+ mock_session.listTags.return_value = mock_tags`
		`+ mock_get_session.return_value = mock_session`
		`+`
		`+ rpm = quote('module-build-macros-0.1-1.testmodule_master_20170303190726.src.rpm')`
		`+ rv = self.client.get('/module-build-service/1/module-builds/?rpm=%s' % rpm)`
		`+ results = json.loads(rv.data)['items']`
		`+`
		`+ assert len(results) == 2`
		`+ assert results[0]["koji_tag"] == "module-testmodule-master-20170219191323-c40c156c"`
		`+ assert results[1]["koji_tag"] == "module-testmodule-master-20170109091357-78e4a6fd"`
		`+`
		`+ mock_session.getRPM.assert_called_once_with(`
		`+ "module-build-macros-0.1-1.testmodule_master_20170303190726.src.rpm")`
		`+ mock_session.listTags.assert_called_once_with(mock_rpm_md["build_id"])`
		`+`
		`+ @patch('module_build_service.config.Config.system',`
		`+ new_callable=PropertyMock, return_value="invalid_builder")`
		`+ def test_query_builds_with_binary_rpm_not_koji(self, mock_builder):`
		`+ rpm = quote('module-build-macros-0.1-1.testmodule_master_20170303190726.src.rpm')`
		`+ rv = self.client.get('/module-build-service/1/module-builds/?rpm=%s' % rpm)`
		`+ results = json.loads(rv.data)`
		`+ expected_error = {`
		`+ 'error': 'Bad Request',`
		`+ 'message': 'Configured builder does not allow to search by rpm binary name!',`
		`+ 'status': 400`
		`+ }`
		`+ assert rv.status_code == 400`
		`+ assert results == expected_error`
		`+`
		`def test_query_component_build(self):`
		`rv = self.client.get('/module-build-service/1/component-builds/1')`
		`data = json.loads(rv.data)`

mcurlej commented 5 years ago

Hi All,

we spoke about this with @jkaluza before he went to PTO. We use koji to find the rpm and then get the tags. After that we can find the correct module in mbs, to which an rpm belongs to, by its tag.

WDYT?
@ralph @fivaldi @mprahl

Signed-off-by: Martin Curlej mcurlej@redhat.com

fivaldi commented on line 24 of module_build_service/utils/views.py 5 years ago

I'd suggest using static method get session from builder.KojiModuleBuilder.KojiModuleBuilder.

What do you think?

mprahl commented 5 years ago

@mcurlej, so although we only use one builder (Koji), MBS has a plugin system for different builders. I suggest adding this functionality as a method (likely static) on the Koji builder. That way when COPR support returns (it's been broken for a while), they can specify how to query COPR instead of Koji.

fivaldi commented on line 19 of module_build_service/utils/views.py 5 years ago

The input should be sanitized against some malformed requests.

fivaldi commented 5 years ago

Please document the rpm param in the README.

ralph commented on line 26 of module_build_service/utils/views.py 5 years ago

What should happen if rpm_md is None?

mcurlej commented on line 19 of module_build_service/utils/views.py 5 years ago

@fivaldi this should be done in an another PR as no input is checked. Afaik this is done by flask itself. At least it removes special characters or converts them to url friendly entities, thats why in the test i have to quote because the special chars where removed by flask.

fivaldi commented on line 19 of module_build_service/utils/views.py 5 years ago

OK :thumbsup:

mcurlej commented 5 years ago

@ralph @mprahl @fivaldi

Updated pls review.

rebased onto b53be271e42986cb82f39da2b76076b4f3f76530

5 years ago

mprahl commented on line 7 of module_build_service/builder/KojiModuleBuilder.py 5 years ago

Could you add an entry in the base class for this method and raise NotImplementedError() like we do for the other required method?

Edit:
On second thought, this is very Koji specific so I think it's best to not create an entry in the base class.

Edited 5 years ago by mprahl

mprahl commented on line 7 of module_build_service/builder/KojiModuleBuilder.py 5 years ago

Can you add a docstring for this method?

mprahl commented on line 18 of module_build_service/builder/KojiModuleBuilder.py 5 years ago

What if the first tag is a tag that MBS isn't aware of? I don't think this happens now but it could in the future. You could at the very least make sure the tag starts with module-, but it'd be better if you just returned all the tags that start with module- and don't end with -build and filter based off of those.