diff --git a/ReviewBot.py b/ReviewBot.py index cbf10da9..ca370efa 100644 --- a/ReviewBot.py +++ b/ReviewBot.py @@ -29,7 +29,9 @@ from collections import OrderedDict from osclib.cache import Cache from osclib.comments import CommentAPI from osclib.conf import Config +from osclib.core import devel_project_fallback from osclib.core import group_members +from osclib.core import maintainers_get from osclib.memoize import memoize from osclib.memoize import memoize_session_reset from osclib.stagingapi import StagingAPI @@ -307,6 +309,36 @@ class ReviewBot(object): if code != 'ok': raise Exception('non-ok return code: {}'.format(code)) + def devel_project_review_add(self, request, project, package, message='adding devel project review'): + devel_project, devel_package = devel_project_fallback(self.apiurl, project, package) + if not devel_project: + self.logger.warning('no devel project found for {}/{}'.format(project, package)) + return False + + self.add_review(request, by_project=devel_project, by_package=devel_package, msg=message) + return True + + def devel_project_review_ensure(self, request, project, package, message='submitter not devel maintainer'): + if not self.devel_project_review_needed(request, project, package): + self.logger.debug('devel project review not needed') + return True + + return self.devel_project_review_add(request, project, package, message) + + def devel_project_review_needed(self, request, project, package): + author = request.get_creator() + maintainers = set(maintainers_get(self.apiurl, project, package)) + + if author in maintainers: + return False + + # Carried over from maintbot, but seems haphazard. + for review in request.reviews: + if review.by_user in maintainers: + return False + + return True + def check_one_request(self, req): """ check all actions in one request. diff --git a/check_source.py b/check_source.py index cef93460..e1355e0b 100755 --- a/check_source.py +++ b/check_source.py @@ -18,7 +18,6 @@ from osclib.core import devel_project_get from osclib.core import devel_project_fallback import urllib2 import ReviewBot -from check_maintenance_incidents import MaintenanceChecker from osclib.conf import str2bool class CheckSource(ReviewBot.ReviewBot): @@ -31,8 +30,6 @@ class CheckSource(ReviewBot.ReviewBot): # ReviewBot options. self.request_default_return = True - self.maintbot = MaintenanceChecker(*args, **kwargs) - self.skip_add_reviews = False def target_project_config(self, project): @@ -286,8 +283,7 @@ class CheckSource(ReviewBot.ReviewBot): links = root.findall('sourceinfo/linked') if links is None or len(links) == 0: if not self.ignore_devel: - # Utilize maintbot to add devel project review if necessary. - self.maintbot.check_one_request(request) + self.devel_project_review_ensure(request, action.tgt_project, action.tgt_package) if not self.skip_add_reviews and self.repo_checker is not None: self.add_review(self.request, by_user=self.repo_checker, msg='Is this delete request safe?') diff --git a/dist/package/openSUSE-release-tools.spec b/dist/package/openSUSE-release-tools.spec index 66fc8039..046dca12 100644 --- a/dist/package/openSUSE-release-tools.spec +++ b/dist/package/openSUSE-release-tools.spec @@ -133,8 +133,6 @@ Group: Development/Tools/Other BuildArch: noarch Requires: %{name} = %{version} Requires: osclib = %{version} -# osrt-leaper-review needs check_maintenance_incidents.py -Requires: %{name}-maintenance = %{version} Requires(pre): shadow %description leaper diff --git a/leaper.py b/leaper.py index 3c612a95..f5e29876 100755 --- a/leaper.py +++ b/leaper.py @@ -39,7 +39,6 @@ from osclib.core import devel_project_get import urllib2 import yaml import ReviewBot -from check_maintenance_incidents import MaintenanceChecker from check_source_in_factory import FactorySourceChecker class Leaper(ReviewBot.ReviewBot): @@ -53,7 +52,6 @@ class Leaper(ReviewBot.ReviewBot): self.do_comments = True - self.maintbot = MaintenanceChecker(*args, **kwargs) # for FactorySourceChecker self.factory = FactorySourceChecker(*args, **kwargs) @@ -125,6 +123,26 @@ class Leaper(ReviewBot.ReviewBot): return project.startswith(origin) def check_source_submission(self, src_project, src_package, src_rev, target_project, target_package): + ret = self.check_source_submission_inner(src_project, src_package, src_rev, target_project, target_package) + + # The layout of this code is just plain wrong and awkward. What is + # really desired a "post check source submission" not + # check_one_request() which applies to all action types and all actions + # at once. The follow-ups need the same context that the main method + # determining to flip the switch had. For maintenance incidents the + # ReviewBot code does a fair bit of mangling to the package and project + # values passed in which is not present during check_one_request(). + # Currently the only feature used by maintenance is + # do_check_maintainer_review and the rest of the checks apply in the + # single action workflow so this should work fine, but really all the + # processing should be done per-action instead of per-request. + + if self.do_check_maintainer_review: + self.devel_project_review_ensure(self.request, target_project, target_package) + + return ret + + def check_source_submission_inner(self, src_project, src_package, src_rev, target_project, target_package): super(Leaper, self).check_source_submission(src_project, src_package, src_rev, target_project, target_package) self.automatic_submission = False @@ -228,6 +246,9 @@ class Leaper(ReviewBot.ReviewBot): self.logger.info("expected origin is '%s' (%s)", origin, "unchanged" if origin_same else "changed") + # Only when not from current product should request require maintainer review. + self.do_check_maintainer_review = False + if origin_same: return True @@ -240,6 +261,8 @@ class Leaper(ReviewBot.ReviewBot): self.logger.info('found pending submission against origin ({})'.format(origin)) return good + self.do_check_maintainer_review = True + return None elif self.action.type == 'maintenance_incident': self.logger.debug('unhandled incident pattern (targetting non :Update project)') @@ -478,9 +501,6 @@ class Leaper(ReviewBot.ReviewBot): self.packages = {} request_ok = ReviewBot.ReviewBot.check_one_request(self, req) - if self.do_check_maintainer_review: - has_correct_maintainer = self.maintbot.check_one_request(req) - self.logger.debug("has_correct_maintainer: %s", has_correct_maintainer) self.logger.debug("review result: %s", request_ok) if self.pending_factory_submission: