From f2375398b8c408c797b9620a90f41d9fddd9cd88 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Apr 09 2019 08:17:07 +0000 Subject: [PATCH 1/4] [frontend] fix Pagure-triggered Package builds There might exist multiple Packages in database (across many coprs) being build-triggered by the same clone_url (by the same pagure pull-request event). But previously would be triggered only one package build, for the first Package (randomly) because the code expected that only one Build in whole database can reference the same 'scm_object_url'. The most frequent problem with this was in forked projects probably; project forks inherited packages with theirs SCM configuration. There was also bug in packages_logic.py where the old code expected that 'CoprDir.name' is unique across all copr projects. So instead of filtering by name, filter by id. Fixes: #586, PR#641 --- diff --git a/frontend/coprs_frontend/coprs/logic/packages_logic.py b/frontend/coprs_frontend/coprs/logic/packages_logic.py index 8199c42..5b3ddcc 100644 --- a/frontend/coprs_frontend/coprs/logic/packages_logic.py +++ b/frontend/coprs_frontend/coprs/logic/packages_logic.py @@ -119,13 +119,15 @@ WHERE package.copr_dir_id = :copr_dir_id; models.Package.name == package_name) @classmethod - def get_by_dir_name(cls, copr_dir_name, package_name): + def get_by_dir(cls, copr_dir, package_name): return models.Package.query.join(models.CoprDir).filter( - models.CoprDir.name == copr_dir_name, models.Package.name == package_name) + models.CoprDir.id==copr_dir.id, + models.Package.name==package_name + ) @classmethod def get_or_create(cls, copr_dir, package_name, src_pkg): - package = cls.get_by_dir_name(copr_dir.name, package_name).first() + package = cls.get_by_dir(copr_dir, package_name).first() if package: return package diff --git a/frontend/coprs_frontend/run/pagure-events.py b/frontend/coprs_frontend/run/pagure-events.py index 21a263c..beeb242 100755 --- a/frontend/coprs_frontend/run/pagure-events.py +++ b/frontend/coprs_frontend/run/pagure-events.py @@ -55,16 +55,13 @@ TOPICS = { class ScmPackage(object): def __init__(self, db_row): - self.pkg_id = db_row.package_id - self.copr_id = db_row.copr_id - self.source_json_dict = json.loads(db_row.source_json) self.clone_url = self.source_json_dict.get('clone_url') or '' self.committish = self.source_json_dict.get('committish') or '' self.subdirectory = self.source_json_dict.get('subdirectory') or '' - self.package = ComplexLogic.get_package_by_id_safe(self.pkg_id) - self.copr = ComplexLogic.get_copr_by_id_safe(self.copr_id) + self.package = ComplexLogic.get_package_by_id_safe(db_row.package_id) + self.copr = self.package.copr def build(self, source_dict_update, copr_dir, update_callback, scm_object_type, scm_object_id, scm_object_url): @@ -75,7 +72,10 @@ class ScmPackage(object): package = self.package db.session.execute('LOCK TABLE build IN EXCLUSIVE MODE') - if models.Build.query.filter(models.Build.scm_object_url == scm_object_url).first(): + query = (models.Build.query + .filter(models.Build.scm_object_url==scm_object_url) + .filter(models.Build.package_id==package.id)) + if db.session.query(query.exists()).scalar(): log.info('\t -> Build for {} already exists.'.format(scm_object_url)) return None @@ -259,7 +259,13 @@ def build_on_fedmsg_loop(): log.error('Bad http status {0} from url {1}'.format(r.status_code, raw_commit_url)) for pkg in candidates: - log.info('Considering pkg id: {}, source_json: {}'.format(pkg.pkg_id, pkg.source_json_dict)) + package = '{}/{}(id={})'.format( + pkg.package.copr.full_name, + pkg.package.name, + pkg.package.id + ) + log.info('Considering pkg package: {}, source_json: {}' + .format(package, pkg.source_json_dict)) if (git_compare_urls(pkg.clone_url, event_info.base_clone_url) and (not pkg.committish or event_info.branch_to.endswith(pkg.committish)) From 3363b743bd82809e1abe995464107adfb1acd860 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Apr 09 2019 08:17:07 +0000 Subject: [PATCH 2/4] [frontend] better parse Pagure's PR messages Pull-requests in Pagure nowadays send also '*.pull-request.updated' and '*.pull-request.rebased' messages, so let's use those instead of 'pull-request.comment.added'. To allow repeated builds per one pull-request, we don't store pull-request URL to database anymore (we store there a unique location of object for each push), so it is not easy to reference the pull request in Pagure UI from Copr UI anymore. We can fix this later if anyone considers this to be important. Fixes: #586, PR#641 --- diff --git a/frontend/coprs_frontend/coprs/templates/coprs/detail/build.html b/frontend/coprs_frontend/coprs/templates/coprs/detail/build.html index 52374e1..2913266 100644 --- a/frontend/coprs_frontend/coprs/templates/coprs/detail/build.html +++ b/frontend/coprs_frontend/coprs/templates/coprs/detail/build.html @@ -95,9 +95,7 @@ {% if build.scm_object_type == 'pull-request' %}
Pull Request:
- - #{{ build.scm_object_id }} - + PR#{{ build.scm_object_id }}
{% endif %}
Directory:
diff --git a/frontend/coprs_frontend/run/pagure-events.py b/frontend/coprs_frontend/run/pagure-events.py index beeb242..1adcd6d 100755 --- a/frontend/coprs_frontend/run/pagure-events.py +++ b/frontend/coprs_frontend/run/pagure-events.py @@ -40,18 +40,25 @@ else: log.info("ENDPOINT = {}".format(ENDPOINT)) -TOPICS = { - 'io.pagure.prod.pagure.git.receive': 'https://pagure.io/', - 'io.pagure.prod.pagure.pull-request.new': 'https://pagure.io/', - 'io.pagure.prod.pagure.pull-request.comment.added': 'https://pagure.io/', - 'org.fedoraproject.prod.pagure.git.receive': 'https://src.fedoraproject.org/', - 'org.fedoraproject.prod.pagure.pull-request.new': 'https://src.fedoraproject.org/', - 'org.fedoraproject.prod.pagure.pull-request.comment.added': 'https://src.fedoraproject.org/', - 'io.pagure.stg.pagure.git.receive': 'https://stg.pagure.io/', # testing only - 'io.pagure.stg.pagure.pull-request.new': 'https://stg.pagure.io/', # testing only - 'io.pagure.stg.pagure.pull-request.comment.added': 'https://stg.pagure.io/', # testing only +pagure_instances = { + 'https://pagure.io/': 'io.pagure.prod.pagure', + 'https://src.fedoraproject.org/': 'org.fedoraproject.prod.pagure', + 'https://stg.pagure.io/': 'io.pagure.stg.pagure', # testing only } +topics = [ + 'git.receive', + 'pull-request.new', + 'pull-request.rebased', + 'pull-request.updated', + 'pull-request.comment.added', +] + +TOPICS = {} +for url, fedmsg_prefix in pagure_instances.items(): + for topic in topics: + TOPICS['{0}.{1}'.format(fedmsg_prefix, topic)] = url + class ScmPackage(object): def __init__(self, db_row): @@ -116,7 +123,7 @@ class ScmPackage(object): return False -def event_info_from_pr_update(data, base_url): +def event_info_from_pr_comment(data, base_url): """ Message handler for updated pull-request opened in pagure. Topic: ``*.pagure.pull-request.comment.added`` @@ -134,23 +141,12 @@ def event_info_from_pr_update(data, base_url): log.info('Comment was not a notification, discarding.') return False - return munch.Munch({ - 'object_id': data['msg']['pullrequest']['id'], - 'object_type': 'pull-request', - 'base_project_url_path': data['msg']['pullrequest']['project']['url_path'], - 'base_clone_url_path': data['msg']['pullrequest']['project']['fullname'], - 'base_clone_url': base_url + data['msg']['pullrequest']['project']['fullname'], - 'project_url_path': data['msg']['pullrequest']['repo_from']['url_path'], - 'clone_url_path': data['msg']['pullrequest']['repo_from']['fullname'], - 'clone_url': base_url + data['msg']['pullrequest']['repo_from']['fullname'], - 'branch_from': data['msg']['pullrequest']['branch_from'], - 'branch_to': data['msg']['pullrequest']['branch'], - 'start_commit': data['msg']['pullrequest']['commit_start'], - 'end_commit': data['msg']['pullrequest']['commit_stop'], - }) + log.info("We don't handle PR coments for now") + # TODO: we could accept e.g. '[copr-test]' here + return False -def event_info_from_new_pr(data, base_url): +def event_info_from_pr(data, base_url): """ Message handler for new pull-request opened in pagure. Topic: ``*.pagure.pull-request.new`` @@ -233,10 +229,10 @@ def build_on_fedmsg_loop(): log.error('Unknown topic {} received. Continuing.') continue - if re.match(r'^.*.pull-request.new$', data['topic']): - event_info = event_info_from_new_pr(data, base_url) + if re.match(r'^.*.pull-request.(new|rebased|updated)$', data['topic']): + event_info = event_info_from_pr(data, base_url) elif re.match(r'^.*.pull-request.comment.added$', data['topic']): - event_info = event_info_from_pr_update(data, base_url) + event_info = event_info_from_pr_comment(data, base_url) else: event_info = event_info_from_push(data, base_url) @@ -277,8 +273,8 @@ def build_on_fedmsg_loop(): dirname = pkg.copr.name + ':pr:' + str(event_info.object_id) copr_dir = CoprDirsLogic.get_or_create(pkg.copr, dirname) update_callback = 'pagure_flag_pull_request' - scm_object_url = os.path.join(base_url, event_info.base_project_url_path, - 'pull-request', str(event_info.object_id)) + scm_object_url = os.path.join(base_url, event_info.project_url_path, + 'c', str(event_info.end_commit)) else: copr_dir = pkg.copr.main_dir update_callback = 'pagure_flag_commit' From 0ac135dd0b18cf5a4a0431b9ccf319b9be3c6aee Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Apr 09 2019 08:17:07 +0000 Subject: [PATCH 3/4] [frontend] pagure-events: allow duplicate builds Since we don't build packages for each 'pull-request.comment.added' message, it doesn't seem to be useful to limit copr builds for unique git objects. --- diff --git a/frontend/coprs_frontend/run/pagure-events.py b/frontend/coprs_frontend/run/pagure-events.py index 1adcd6d..fc4cc2c 100755 --- a/frontend/coprs_frontend/run/pagure-events.py +++ b/frontend/coprs_frontend/run/pagure-events.py @@ -79,13 +79,6 @@ class ScmPackage(object): package = self.package db.session.execute('LOCK TABLE build IN EXCLUSIVE MODE') - query = (models.Build.query - .filter(models.Build.scm_object_url==scm_object_url) - .filter(models.Build.package_id==package.id)) - if db.session.query(query.exists()).scalar(): - log.info('\t -> Build for {} already exists.'.format(scm_object_url)) - return None - return BuildsLogic.rebuild_package( package, source_dict_update, copr_dir, update_callback, scm_object_type, scm_object_id, scm_object_url) From 2d1b9ff10679cc9ac0dcfefd894d735ab606c6ea Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Apr 09 2019 08:17:07 +0000 Subject: [PATCH 4/4] [frontend] pagure-events: accept [copr-build] key in PR message --- diff --git a/frontend/coprs_frontend/run/pagure-events.py b/frontend/coprs_frontend/run/pagure-events.py index fc4cc2c..ddf54a6 100755 --- a/frontend/coprs_frontend/run/pagure-events.py +++ b/frontend/coprs_frontend/run/pagure-events.py @@ -130,13 +130,28 @@ def event_info_from_pr_comment(data, base_url): return False last_comment = data['msg']['pullrequest']['comments'][-1] - if not last_comment or last_comment['notification'] is False: - log.info('Comment was not a notification, discarding.') + if not last_comment: + log.info('Can not access last comment, discarding.') return False - log.info("We don't handle PR coments for now") - # TODO: we could accept e.g. '[copr-test]' here - return False + if not 'comment' in last_comment or '[copr-build]' not in last_comment['comment']: + log.info('The [copr-build] is not present in the message.') + return False + + return munch.Munch({ + 'object_id': data['msg']['pullrequest']['id'], + 'object_type': 'pull-request', + 'base_project_url_path': data['msg']['pullrequest']['project']['url_path'], + 'base_clone_url_path': data['msg']['pullrequest']['project']['fullname'], + 'base_clone_url': base_url + data['msg']['pullrequest']['project']['fullname'], + 'project_url_path': data['msg']['pullrequest']['repo_from']['url_path'], + 'clone_url_path': data['msg']['pullrequest']['repo_from']['fullname'], + 'clone_url': base_url + data['msg']['pullrequest']['repo_from']['fullname'], + 'branch_from': data['msg']['pullrequest']['branch_from'], + 'branch_to': data['msg']['pullrequest']['branch'], + 'start_commit': data['msg']['pullrequest']['commit_start'], + 'end_commit': data['msg']['pullrequest']['commit_stop'], + }) def event_info_from_pr(data, base_url):