From 1e09f939906cce56d88fe3dfe29909673eb880f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Thu, 22 Jun 2023 17:48:18 +0200 Subject: [PATCH 1/2] Add type hints to action_ methods of ReviewBot --- ReviewBot.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ReviewBot.py b/ReviewBot.py index 0fef868b0..14cf40f38 100644 --- a/ReviewBot.py +++ b/ReviewBot.py @@ -4,6 +4,7 @@ import sys import re import logging +from typing import List import cmdln from collections import namedtuple from collections import OrderedDict @@ -95,7 +96,7 @@ def __init__(self, apiurl=None, dryrun=False, logger=None, user=None, group=None self.logger = logger self.review_user = user self.review_group = group - self.requests = [] + self.requests: List[osc.core.Request] = [] self.review_messages = ReviewBot.DEFAULT_REVIEW_MESSAGES self._review_mode = 'normal' self.fallback_user = None @@ -398,7 +399,7 @@ def devel_project_review_needed(self, request, project, package): return True - def check_one_request(self, req): + def check_one_request(self, req: osc.core.Request): """ check all actions in one request. @@ -427,11 +428,12 @@ def check_one_request(self, req): overall = True for a in req.actions: + a: osc.core.Action if self.multiple_actions: self.review_messages = self.DEFAULT_REVIEW_MESSAGES.copy() # Store in-case sub-classes need direct access to original values. - self.action = a + self.action: osc.core.Action = a key = request_action_key(a) override = self.request_override_check() @@ -462,7 +464,7 @@ def check_one_request(self, req): return overall - def action_method(self, action): + def action_method(self, action: osc.core.Action): method_prefix = 'check_action' method_type = action.type method_suffix = None @@ -511,7 +513,7 @@ def check_action_maintenance_incident(self, req, a): return self.check_source_submission(a.src_project, a.src_package, a.src_rev, a.tgt_releaseproject, tgt_package) - def check_action_maintenance_release(self, req, a): + def check_action_maintenance_release(self, req: osc.core.Request, a: osc.core.Action): pkgname = a.src_package if action_is_patchinfo(a): self.logger.debug('ignoring patchinfo action') @@ -530,7 +532,7 @@ def check_action_maintenance_release(self, req, a): pkgname = linkpkg return self.check_source_submission(a.src_project, a.src_package, None, a.tgt_project, pkgname) - def check_action_submit(self, req, a): + def check_action_submit(self, req: osc.core.Request, a: osc.core.Action): return self.check_source_submission(a.src_project, a.src_package, a.src_rev, a.tgt_project, a.tgt_package) def check_action__default(self, req, a): From 5470fa4fbe07ec7c9781f6f09fbbfaf2d5d3bc6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Thu, 22 Jun 2023 10:39:21 +0200 Subject: [PATCH 2/2] Don't decline SRs from non-devel project for scmsync packages from the bot We want to start transitioning to a git based development workflow. For the first iteration, we would allow maintainers to opt-in by changing their package in the devel project to use scmsync from src.opensuse.org/pool/${pkg_name} and submit changes via pull requests on gitea. These pull requests will get submitted directly by the scm-staging-bot to Factory from its home project as submit requests. Currently, such a submission would get auto-declined from the factory-auto bot. With this commit, factory-auto will no longer decline such a submission provided that the above conditions have been met. Co-authored-by: Dirk Mueller --- check_source.py | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/check_source.py b/check_source.py index 1b97b6d13..cd1c90e76 100755 --- a/check_source.py +++ b/check_source.py @@ -21,7 +21,7 @@ from osclib.core import package_kind from osclib.core import create_add_role_request from osclib.core import maintainers_get -from osc.core import show_project_meta +from osc.core import show_package_meta, show_project_meta from osc.core import get_request_list from urllib.error import HTTPError @@ -62,6 +62,7 @@ def target_project_config(self, project): self.allow_valid_source_origin = str2bool(config.get('check-source-allow-valid-source-origin', 'False')) self.valid_source_origins = set(config.get('check-source-valid-source-origins', '').split(' ')) self.add_devel_project_review = str2bool(config.get('check-source-add-devel-project-review', 'False')) + self.allowed_scm_submission_sources = set(config.get('allowed-scm-submission-sources', '').split(' ')) if self.action.type == 'maintenance_incident': # The workflow effectively enforces the names to match and the @@ -150,12 +151,25 @@ def check_source_submission(self, source_project, source_package, source_revisio self.logger.info('checking if target package exists and has devel project') devel_project, devel_package = devel_project_get(self.apiurl, target_project, target_package) if devel_project: - if ((source_project != devel_project or source_package != devel_package) and - not (source_project == target_project and source_package == target_package)): - # Not from proper devel project/package and not self-submission. - self.review_messages['declined'] = 'Expected submission from devel package %s/%s' % ( - devel_project, devel_package) - return False + if ( + (source_project != devel_project or source_package != devel_package) + and not (source_project == target_project and source_package == target_package)): + # check if the devel project & package are using scmsync & match the allowed prj prefix + # => waive the devel project source submission requirement + meta = ET.fromstringlist(show_package_meta(self.apiurl, devel_project, devel_package)) + scm_sync = meta.find('scmsync') + if ( + not scm_sync or + ( + scm_sync and + not scm_sync.text.startswith(f"https://src.opensuse.org/pool/{source_package}") + and all(not source_project.startswith(allowed_src) for allowed_src in self.allowed_scm_submission_sources) + ) + ): + # Not from proper devel project/package and not self-submission and not scmsync. + self.review_messages['declined'] = 'Expected submission from devel package %s/%s' % ( + devel_project, devel_package) + return False else: # Check to see if other packages exist with the same source project # which indicates that the project has already been used as devel.