PR#15: Two minor improvements - ursa-major

ursa-major

#15 Two minor improvements

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

cqi/ursa-major minor-fixes into master

Check module build state in the beginning of add tag handler run

Chenxiong Qi • 5 years ago

b73ee55

Some fixes to funtion requires_included

Chenxiong Qi • 5 years ago

22bda31

Use Logger.warning instead of Logger.warn

Chenxiong Qi • 5 years ago

67ef53d

tests/test_koji_service.py

file modified

+3 -3

		`@@ -320,7 +320,7 @@`
		`add_tags = [{'name': 'module-123', 'priority': 30}]`
		`self.koji.update_tag_inheritance('f27-build', add_tags)`

		`- log.warn.assert_called_with(`
		`+ log.warning.assert_called_with(`
		`("Tag f27-build has an inheritance with the same priority 30 as "`
		`"tag module-123"))`

		`@@ -470,7 +470,7 @@`

		`self.koji.koji_proxy.newRepo.assert_called_with(`
		`'rhel-8.0-build', debuginfo=True, src=True)`
		`- log.warn.assert_has_calls(`
		`+ log.warning.assert_has_calls(`
		`[mock.call('Warning: %s is not a build tag', 'rhel-8.0-build'),`
		`mock.call('Warning: tag %s has an empty arch list',`
		`'rhel-8.0-build')],`
		`@@ -505,7 +505,7 @@`

		`self.koji.koji_proxy.newRepo.assert_called_with(`
		`'rhel-8.0-build', debuginfo=True, src=True)`
		`- log.warn.assert_has_calls(`
		`+ log.warning.assert_has_calls(`
		`[mock.call('Warning: %s is not a build tag', 'rhel-8.0-build'),`
		`mock.call('Warning: tag %s has an empty arch list',`
		`'rhel-8.0-build')],`

ursa_major/handlers/add_module.py

file modified

+12 -12

		`@@ -60,14 +60,14 @@`

		`if self.args.update_inheritance_only:`
		`to_update_config = False`
		`- log.warn("Will not update config file because --update-inheritance-only "`
		`- "is specified, even if the specified module config does not "`
		`- "exist in tag config file.")`
		`+ log.warning("Will not update config file because --update-inheritance-only "`
		`+ "is specified, even if the specified module config does not "`
		`+ "exist in tag config file.")`
		`if self.args.update_config_only:`
		`to_update_inheritance = False`
		`- log.warn("Will not update tag %s's inheritance data because --update-config-only "`
		`- "is specified, even if the latest module build's tag does not exists in "`
		`- "inheritance with specified config", self.op_tag)`
		`+ log.warning("Will not update tag %s's inheritance data because --update-config-only "`
		`+ "is specified, even if the latest module build's tag does not exists in "`
		`+ "inheritance with specified config", self.op_tag)`

		`if self.args.module_requires:`
		`module_requires = dict(self.args.module_requires)`
		`@@ -109,12 +109,12 @@`
		`tag_to_add = latest_module_tag`

		`else:`
		`- log.warn("Can't find any built module in MBS with module config '%s', "`
		`- "we will not add any new module tag to inheritance data",`
		`- input_module_config)`
		`+ log.warning("Can't find any built module in MBS with module config '%s', "`
		`+ "we will not add any new module tag to inheritance data",`
		`+ input_module_config)`
		`if not self.force:`
		`- log.warn("Will not update inheritance data nor tag config file, consider "`
		`- "to run with --force to allow updating")`
		`+ log.warning("Will not update inheritance data nor tag config file, consider "`
		`+ "to run with --force to allow updating")`
		`to_update_inheritance = False`
		`to_update_config = False`

		`@@ -255,7 +255,7 @@`
		`build_tags = self.koji.find_build_tags(self.op_tag)`

		`if not build_tags:`
		`- log.warn("No build tag found for %s, not regen any repo", self.op_tag)`
		`+ log.warning("No build tag found for %s, not regen any repo", self.op_tag)`
		`else:`
		`log.info("Found build tags for %s: %r", self.op_tag, build_tags)`
		`self.koji.regen_repos(build_tags, wait=self.wait_regen_repo)`

ursa_major/handlers/add_tag.py

file modified

+19 -21

		`@@ -167,7 +167,7 @@`
		`build_tags = self.koji.find_build_tags(tag)`

		`if not build_tags:`
		`- log.warn("No build tag found for %s, not regen any repo", tag)`
		`+ log.warning("No build tag found for %s, not regen any repo", tag)`
		`else:`
		`log.info("Found build tags for '%s': %r", tag, build_tags)`
		`self.koji.regen_repos(build_tags, wait=self.wait_regen_repo)`
		`@@ -175,10 +175,24 @@`
		`return [modinfo.koji_tag], old_tags`

		`def run(self):`
		`+ try:`
		`+ ci_message_string = get_env_var(self.args.module_from_env,`
		`+ raise_if_not_exist=True)`
		`+ ci_message = json.loads(ci_message_string)`
		`+ log.info('Received MBS state change message:\n%s',`
		`+ json.dumps(ci_message, indent=2, sort_keys=True))`
		`+`
		`+ except Exception as e:`
		`+ error = "Fail to load '{!s}' from os environ: {!s}".format(self.args.module_from_env, e)`
		`+ log.error(error)`
		`+ raise RuntimeError(error)`
		`+`
		`+ if ci_message.get('state_name', None) != 'ready':`
		`+ log.info("MBS build state is not 'ready', skipping")`
		`+ return`
		`+`
		`self.dry_run = self.args.dry_run`
		`self.send_mail = self.args.send_mail`
		`- self.module_from_env = self.args.module_from_env`
		`-`
		`self.tag_config_file = self.args.tag_config_file`
		`self.wait_regen_repo = self.args.wait_regen_repo`

		`@@ -195,22 +209,6 @@`
		`self.connect_koji(dry_run=self.dry_run)`
		`self.connect_mbs()`

		`- try:`
		`- ci_message_string = get_env_var(self.module_from_env,`
		`- raise_if_not_exist=True)`
		`- ci_message = json.loads(ci_message_string)`
		`- log.info('Received MBS state change message:\n%s',`
		`- json.dumps(ci_message, indent=2, sort_keys=True))`
		`-`
		`- except Exception as e:`
		`- error = "Fail to load '{!s}' from os environ: {!s}".format(self.module_from_env, e)`
		`- log.error(error)`
		`- raise RuntimeError(error)`
		`-`
		`- if ci_message.get('state_name', None) != 'ready':`
		`- log.info("MBS build state is not 'ready', skipping")`
		`- return`
		`-`
		`modinfo = ModuleInfo.from_mbs_message(self.mbs, ci_message)`
		`if self.send_mail:`
		`self.mail_data['module'] = modinfo.short_info`
		`@@ -219,8 +217,8 @@`
		`for tag, tag_config in self.tag_config.items():`
		`tag_owners = tag_config.get('owners', [])`
		`if not tag_owners:`
		`- log.warn("Sending mail is enabled, but no owners set for tag %s, "`
		`- "will not send mail", tag)`
		`+ log.warning("Sending mail is enabled, but no owners set for tag %s, "`
		`+ "will not send mail", tag)`

		`conf_modules = tag_config.get('modules', [])`
		`try:`

ursa_major/handlers/check_config.py

file modified

+3 -3

		`@@ -74,14 +74,14 @@`

		`tag_owners = config.get('owners', None)`
		`if tag_owners is None or not tag_owners:`
		`- log.warn("Tag %s: missing owners", tag)`
		`+ log.warning("Tag %s: missing owners", tag)`
		`if not isinstance(tag_owners, list):`
		`- log.warn("Tag %s: owners should be a list of emails", tag)`
		`+ log.warning("Tag %s: owners should be a list of emails", tag)`

		`module_configs = config.get('modules', [])`

		`if not module_configs:`
		`- log.warn("Tag %s: no module specified", tag)`
		`+ log.warning("Tag %s: no module specified", tag)`
		`continue`

		`# save modules by priority for checking duplicate priorities`

ursa_major/handlers/remove_module.py

file modified

+2 -2

		`@@ -48,8 +48,8 @@`

		`self.load_tag_config()`
		`if self.tag not in self.tag_config.keys():`
		`- log.warn("Tag %s is not present in tag config file %s",`
		`- self.tag, self.tag_config_file)`
		`+ log.warning("Tag %s is not present in tag config file %s",`
		`+ self.tag, self.tag_config_file)`

		`try:`
		`tag_inheritance = self.koji.get_inheritance_data(self.tag)`

ursa_major/koji_service.py

file modified

+4 -4

		`@@ -364,7 +364,7 @@`
		`"{} as tag {}".format(child_tag['name'],`
		`add_data['priority'],`
		`add_tag['name']))`
		`- log.warn(msg)`
		`+ log.warning(msg)`

		`same_parent = [d for d in inheritance_data if`
		`d['parent_id'] == add_data['parent_id'] and`
		`@@ -438,11 +438,11 @@`
		`tagid = taginfo['id']`
		`tagname = taginfo['name']`
		`if not taginfo['arches']:`
		`- log.warn("Warning: tag %s has an empty arch list", tagname)`
		`+ log.warning("Warning: tag %s has an empty arch list", tagname)`

		`targets = self.get_build_targets(tagid)`
		`if not targets:`
		`- log.warn("Warning: %s is not a build tag", tagname)`
		`+ log.warning("Warning: %s is not a build tag", tagname)`

		`repo_opts = {}`
		`if debuginfo:`
		`@@ -496,7 +496,7 @@`
		`try:`
		`future.result()`
		`except Exception as exc:`
		`- log.warn("Error while re-generating repo for %s: %s", tag, str(exc))`
		`+ log.warning("Error while re-generating repo for %s: %s", tag, str(exc))`
		`else:`
		`if not wait:`
		`log.info("Regen-repo task of tag %s is created", tag)`

ursa_major/utils.py

file modified

+11 -12

		`@@ -91,9 +91,10 @@`
		`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 mmd_requires: a mapping representing either buildrequires or`
		+ requires, which is returned from function ``peek_requires`` and
		+ ``peek_buildrequires``.
		`+ :type mmd_requires: dict[str, Modulemd.SimpleSet]`
		`: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``.
		`@@ -102,21 +103,19 @@`
		`:rtype: bool`
		`"""`
		`for req_name, req_streams in config_requires.items():`
		`- if req_name not in mmd_requires.keys():`
		`+ if req_name not in mmd_requires:`
		`return False`

		`- if not(isinstance(req_streams, list)):`
		`+ if not isinstance(req_streams, list):`
		`req_streams = [req_streams]`

		`- streams = mmd_requires[req_name].dup()`
		`- neg_reqs = [s[1:] for s in req_streams if s.startswith('-')]`
		`- pos_reqs = [s for s in req_streams if not s.startswith('-')]`
		`+ neg_reqs = set(s[1:] for s in req_streams if s.startswith('-'))`
		`+ pos_reqs = set(s for s in req_streams if not s.startswith('-'))`

		`- # TODO: can mmd contains negative requires like`
		`- # {'platform': ['-f26']} ?`
		`- if set(streams) & set(neg_reqs):`
		`+ streams = set(mmd_requires[req_name].dup())`
		`+ if streams & neg_reqs:`
		`return False`
		`- if pos_reqs and not (set(streams) & set(pos_reqs)):`
		`+ if pos_reqs and not (streams & pos_reqs):`
		`return False`
		`return True`