PR#1: Allow filtering on buildrequires - ursa-major

ursa-major

#1 Allow filtering on buildrequires

Merged 5 years ago by cqi. Opened 5 years ago by cqi.

cqi/ursa-major filter-on-buildrequires into master

Allow filtering on buildrequires

Chenxiong Qi • 5 years ago

8d88e4a

README.rst

file modified

+8 -4

		`@@ -22,6 +22,9 @@`
		`{`
		`"name": "httpd",`
		`"priority": 10,`
		`+ "buildrequires": {`
		`+ "platform": "f30"`
		`+ },`
		`"requires": {`
		`"platform": "f30"`
		`},`
		`@@ -57,10 +60,11 @@`

		`A valid module config should contains:`

		- * `name` (required): module name
		- * `stream` (required): module stream
		- * `priority` (required): add module's tag to tag inheritance with this priority
		- * `requires` (optional): module's runtime dependency
		+ * ``name`` (required): module name
		+ * ``stream`` (required): module stream
		+ * ``priority`` (required): add module's tag to tag inheritance with this priority
		+ * ``requires`` (optional): module's runtime dependencies.
		+ * ``buildrequires`` (optional): module's build time dependencies.

		For each tag, ``owners`` can be set with email addresses.

tests/test_add_tag_handler.py

file modified

+107 -61

		`@@ -234,75 +234,121 @@`
		`# messages.`
		`self.assertGreaterEqual(log.info.call_count, 2)`

		`- def test_get_module_config_match_name_stream(self):`
		`- configs = [`
		`- {'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10},`
		`- {'name': 'testmodule2', 'stream': 'rhel-8.0', 'priority': 20}`
		`- ]`
		`- message = self.load_json_from_file("testmodule_ready_message.json")`
		`- mock_mbs = mock.MagicMock()`
		`- fake_requires = {"platform": ["el8"]}`
		`- fake_mmd = self.make_mmd("testmodule", "rhel-8.0",`
		`- "20180409051516", "9e5fe74b",`
		`- requires=fake_requires)`
		`- mock_mbs.get_module_mmd.return_value = fake_mmd`
		`- modinfo = ModuleInfo.from_mbs_message(mock_mbs, message)`
		`- matched_config = AddTagHandler.get_module_config(configs, modinfo)`
		`- expected = {'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10}`
		`- self.assertEqual(matched_config, expected)`
		`-`
		`- def test_get_module_config_match_requires(self):`
		`- configs = [`
		`- {'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10,`
		`- 'requires': {'platform': 'el8'}},`
		`- {'name': 'testmodule2', 'stream': 'rhel-8.0', 'priority': 20}`
		`- ]`
		`+ def assert_get_module_config(self, fake_configs, expected_result):`
		`message_file = "testmodule_with_requires_ready_message.json"`
		`message = self.load_json_from_file(message_file)`
		`mock_mbs = mock.MagicMock()`
		`fake_requires = {"platform": ["el8"]}`
		`fake_mmd = self.make_mmd("testmodule", "rhel-8.0",`
		`"20180409051516", "9e5fe74b",`
		`- requires=fake_requires)`
		`- mock_mbs.get_module_mmd.return_value = fake_mmd`
		`- modinfo = ModuleInfo.from_mbs_message(mock_mbs, message)`
		`- matched_config = AddTagHandler.get_module_config(configs, modinfo)`
		`- expected = {'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10,`
		`- 'requires': {'platform': 'el8'}}`
		`- self.assertEqual(matched_config, expected)`
		`-`
		`- def test_get_module_config_unmatch_stream(self):`
		`- configs = [`
		`- {'name': 'testmodule', 'stream': 'f28', 'priority': 10},`
		`- {'name': 'testmodule2', 'stream': 'rhel-8.0', 'priority': 20}`
		`- ]`
		`- message = self.load_json_from_file("testmodule_ready_message.json")`
		`- mock_mbs = mock.MagicMock()`
		`- fake_requires = {"platform": ["el8"]}`
		`- fake_mmd = self.make_mmd("testmodule", "rhel-8.0",`
		`- "20180409051516", "9e5fe74b",`
		`- requires=fake_requires)`
		`+ requires=fake_requires,`
		`+ buildrequires=fake_requires)`
		`mock_mbs.get_module_mmd.return_value = fake_mmd`
		`modinfo = ModuleInfo.from_mbs_message(mock_mbs, message)`
		`- matched_config = AddTagHandler.get_module_config(configs, modinfo)`
		`- self.assertEqual(matched_config, None)`
		`-`
		`- def test_get_module_config_unmatch_requires(self):`
		`- configs = [`
		`- {'name': 'testmodule', 'stream': 'f28', 'priority': 10,`
		`- 'requires': {'platform': 'el8'}},`
		`- {'name': 'testmodule2', 'stream': 'rhel-8.0', 'priority': 20}`
		`- ]`
		`- message = self.load_json_from_file("testmodule_ready_message.json")`
		`- mock_mbs = mock.MagicMock()`
		`- fake_requires = {"platform": ["el8"]}`
		`- fake_mmd = self.make_mmd("testmodule", "rhel-8.0",`
		`- "20180409051516", "9e5fe74b",`
		`- requires=fake_requires)`
		`- mock_mbs.get_module_mmd.return_value = fake_mmd`
		`- modinfo = ModuleInfo.from_mbs_message(mock_mbs, message)`
		`- matched_config = AddTagHandler.get_module_config(configs, modinfo)`
		`- self.assertEqual(matched_config, None)`
		`+ matched_config = AddTagHandler.get_module_config(fake_configs, modinfo)`
		`+ self.assertEqual(matched_config, expected_result)`
		`+`
		`+ def test_get_module_config_match_requires(self):`
		`+ # ((fake_module_configs, expected result), ...)`
		`+ test_matrix = (`
		`+ (`
		`+ [`
		`+ {`
		`+ 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10,`
		`+ 'requires': {'platform': 'el8'}`
		`+ },`
		`+ # This noisy config should not impact to choose the correct one.`
		`+ {`
		`+ 'name': 'testmodule2', 'stream': 'rhel-8.0', 'priority': 20,`
		`+ }`
		`+ ],`
		`+ {`
		`+ 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10,`
		`+ 'requires': {'platform': 'el8'}`
		`+ },`
		`+ ),`
		`+ (`
		`+ [`
		`+ # buildrequires could be specified in config as well.`
		`+ {`
		`+ 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10,`
		`+ 'buildrequires': {'platform': 'el8'}`
		`+ },`
		`+ ],`
		`+ {`
		`+ 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10,`
		`+ 'buildrequires': {'platform': 'el8'}`
		`+ },`
		`+ ),`
		`+ (`
		`+ [`
		`+ # Both requires and buildrequires are specified to match the`
		`+ # module metadata.`
		`+ {`
		`+ 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10,`
		`+ 'requires': {'platform': 'el8'},`
		`+ 'buildrequires': {'platform': 'el8'},`
		`+ },`
		`+ ],`
		`+ {`
		`+ 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10,`
		`+ 'requires': {'platform': 'el8'},`
		`+ 'buildrequires': {'platform': 'el8'},`
		`+ },`
		`+ ),`
		`+ # Either requires or buildrequires is not included in module metadata.`
		`+ (`
		`+ [`
		`+ {`
		`+ 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10,`
		`+ 'requires': {'platform': 'f30'},`
		`+ },`
		`+ ],`
		`+ None,`
		`+ ),`
		`+ (`
		`+ [`
		`+ # Both requires and buildrequires are specified to match the`
		`+ # module metadata.`
		`+ {`
		`+ 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10,`
		`+ 'buildrequires': {'platform': 'f30'},`
		`+ },`
		`+ ],`
		`+ None,`
		`+ ),`
		`+ (`
		`+ [`
		`+ {`
		`+ 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10,`
		`+ 'requires': {'platform': 'f30'},`
		`+ 'buildrequires': {'platform': 'f30'},`
		`+ },`
		`+ ],`
		`+ None,`
		`+ ),`
		`+ # Either name or stream is not matched module metadata.`
		`+ (`
		`+ # Module name is not matched in this config`
		`+ [{'name': 'module-xxxx', 'stream': 'rhel-8.0', 'priority': 20}],`
		`+ None,`
		`+ ),`
		`+ (`
		`+ # Module stream is not matched in this config`
		`+ [{'name': 'testmodule', 'stream': '100', 'priority': 20}],`
		`+ None,`
		`+ ),`
		`+ # Module name and stream are matched`
		`+ (`
		`+ [`
		`+ {'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 20},`
		`+ {'name': 'testmodule2', 'stream': 'rhel-8.0', 'priority': 20},`
		`+ ],`
		`+ {'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 20},`
		`+ ),`
		`+ )`
		`+ for config, expected_result in test_matrix:`
		`+ self.assert_get_module_config(config, expected_result)`

		`@mock.patch('ursa_major.mbs.requests.get')`
		`def test_get_match_tags_in_inheritance(self, requests_get):`

ursa_major/handlers/add_tag.py

file modified

+10 -2

		`@@ -28,7 +28,7 @@`

		`from ursa_major.logger import log`
		`from ursa_major.mail import MailAPI`
		`- from ursa_major.utils import get_env_var, mmd_has_requires`
		`+ from ursa_major.utils import get_env_var, mmd_has_requires, mmd_has_buildrequires`

		`from ursa_major.handlers.base import BaseHandler`

		`@@ -99,8 +99,13 @@`
		`by the modinfo.`

		`:params module_configs: a list of module config`
		`+ :type module_configs: list[dict]`
		`:params modinfo: instance of ModuleInfo`
		`- :return: an item in module configs, it's a dict.`
		`+ :return: the matched module config. None is returned if no module`
		`+ config matches the module metadata.`
		`+ :rtype: dict`
		`+ :raises RuntimeError: if more than one module configs match the`
		`+ specified module metadata.`
		`"""`
		`matched = []`
		`for config in module_configs:`
		`@@ -108,6 +113,9 @@`
		`stream = config['stream']`
		`if not (name == modinfo.name and stream == modinfo.stream):`
		`continue`
		`+ dep_requires = config.get('buildrequires')`
		`+ if dep_requires and not mmd_has_buildrequires(modinfo.mmd, dep_requires):`
		`+ continue`
		`requires = config.get('requires', {})`
		`if requires and not mmd_has_requires(modinfo.mmd, requires):`
		`continue`

ursa_major/utils.py

file modified

+44 -12

		`@@ -88,19 +88,20 @@`
		`return mmd`


		`- def mmd_has_requires(mmd, requires):`
		`- """`
		`- Check whether a module represent by the mmd has requires.`
		`-`
		`- :param mmd: Modulemd.Module object`
		`- :param requires: dict of requires, example:`
		`- {'platform': 'f28', 'python3': 'master'}`
		`+ def requires_included(mmd_requires, config_requires):`
		`+ """Test if requires defined in config is included in module metadata`
		`+`
		`+ :param dict mmd_requires: a mapping representing either buildrequires or`
		`+ requires, which is generally converted from module metadata. For`
		+ example, ``{"platform": "f29"}``.
		`+ :param dict config_requires: a mapping representing either buildrequires or`
		`+ requires defined in config file. This is what to check if it is`
		+ included in ``mmd_requires``.
		+ :return: True if all requires inside ``config_requires`` are included in
		`+ module metadata. Otherwise, False is returned.`
		`+ :rtype: bool`
		`"""`
		`- deps_list = mmd.peek_dependencies()`
		`- mmd_requires = deps_list[0].peek_requires() if deps_list else {}`
		`-`
		`- neg_reqs = pos_reqs = []`
		`- for req_name, req_streams in requires.items():`
		`+ for req_name, req_streams in config_requires.items():`
		`if req_name not in mmd_requires.keys():`
		`return False`

		`@@ -118,3 +119,34 @@`
		`if pos_reqs and not (set(streams) & set(pos_reqs)):`
		`return False`
		`return True`
		`+`
		`+`
		`+ def mmd_has_requires(mmd, requires):`
		`+ """`
		`+ Check whether a module represent by the mmd has requires.`
		`+`
		`+ :param mmd: Modulemd.Module object`
		`+ :param requires: dict of requires, example:`
		`+ {'platform': 'f28', 'python3': 'master'}`
		`+ """`
		`+ deps_list = mmd.peek_dependencies()`
		`+ mmd_requires = deps_list[0].peek_requires() if deps_list else {}`
		`+ return requires_included(mmd_requires, requires)`
		`+`
		`+`
		`+ def mmd_has_buildrequires(mmd, config_buildrequires):`
		`+ """`
		`+ Check if a module metadata represented by the mmd has buildrequires.`
		`+`
		`+ :param mmd: a module metadata.`
		`+ :type mmd: Modulemd.Module`
		`+ :param dict config_buildrequires: a mapping of buildrequires defined in`
		`+ config file to match the module metadata, for example:`
		+ ``{'platform': 'f28', 'python3': 'master'}``.
		`+ :return: True if the specified module metadata has the buildrequires`
		`+ defined in config.`
		`+ :rtype: bool`
		`+ """`
		`+ deps_list = mmd.peek_dependencies()`
		`+ mmd_requires = deps_list[0].peek_buildrequires() if deps_list else {}`
		`+ return requires_included(mmd_requires, config_buildrequires)`

cqi commented 5 years ago

With this patch, Ursa-Major could be configured to track modules by both
buildrequires and requires. There is an example in README. Meanwhile, just same
as the requires, buildrequires is optional as well.

Signed-off-by: Chenxiong Qi cqi@redhat.com