From a6a45fbfc852c69dd9249826668fd970437de0bf Mon Sep 17 00:00:00 2001 From: Farhaan Bukhsh Date: Jul 31 2016 08:11:56 +0000 Subject: Fix manually entering pagure project name for Pagure CI Service class in hook/jenkins_hook.py is modified in a way that it provides project name on pagure and jenkins, where user doesn't have to manually enter name of the project. Files like consumer.py , lib/pagure_ci.py are adjusted according to the new addition. Forms field for pagure_name is removed since it was not required and similar deletion in database is made since it is no more required. Tests are fixed according to the new changes made. --- diff --git a/pagure/exceptions.py b/pagure/exceptions.py index db3e7fa..3eb83b6 100644 --- a/pagure/exceptions.py +++ b/pagure/exceptions.py @@ -62,3 +62,8 @@ class GitConflictsException(PagureException): class HookInactiveException(PagureException): ''' Exception raised when the hook is inactive. ''' pass + + +class ConfigNotFound(PagureException): + ''' Exception raised when configurations are not found in database. ''' + pass diff --git a/pagure/hooks/jenkins_hook.py b/pagure/hooks/jenkins_hook.py index ad2521c..3dce491 100644 --- a/pagure/hooks/jenkins_hook.py +++ b/pagure/hooks/jenkins_hook.py @@ -16,10 +16,11 @@ from pagure.hooks import BaseHook, RequiredIf from pagure.lib.model import BASE, Project from pagure import get_repo_path from pagure import APP, SESSION +from pagure.exceptions import ConfigNotFound class PagureCI(BASE): - + ''' Table for Pagure CI. ''' __tablename__ = 'hook_pagure_ci' id = sa.Column(sa.Integer, primary_key=True) @@ -34,7 +35,6 @@ class PagureCI(BASE): active = sa.Column(sa.Boolean, nullable=False, default=False) display_name = sa.Column(sa.String(64), nullable=False, default='Jenkins') - pagure_name = sa.Column(sa.String(255)) jenkins_name = sa.Column(sa.String(255)) jenkins_url = sa.Column(sa.String(255), nullable=False, @@ -54,20 +54,21 @@ class PagureCI(BASE): self.pagure_ci_token = uuid.uuid4().hex -class ConfigNotFound(Exception): - pass - - class Service(object): - PAGURE = PagureCI.pagure_name - JENKINS = PagureCI.jenkins_name + ''' Service to provide project name on pagure and jenkins. ''' + JENKINS = None + PAGURE = None + def __init__(self): + self.PAGURE = SESSION.query(Project).filter( + Project.id == PagureCI.project_id).first().name + self.JENKINS = PagureCI.jenkins_name def get_configs(project_name, service): - """Returns all configurations with given name on a service. + ''' Returns all configurations with given name on a service. :raises ConfigNotFound: when no configuration matches - """ + ''' cfg = SESSION.query(PagureCI).filter( service == project_name).all() if not cfg: @@ -76,12 +77,7 @@ def get_configs(project_name, service): class JenkinsForm(wtf.Form): - - '''Form to configure Jenkins hook''' - - pagure_name = TextField('Name of project in Pagure', - [validators.Required(), - validators.Length(max=255)]) + ''' Form to configure Jenkins hook. ''' jenkins_name = TextField('Name of project in Jenkins', [validators.Required(), @@ -99,7 +95,7 @@ class JenkinsForm(wtf.Form): class PagureCiHook(BaseHook): - ''' Jenkins hooks. ''' + ''' Pagure CI hook. ''' name = 'Pagure CI' description = 'This hook help to set up CI for the project'\ @@ -107,10 +103,7 @@ class PagureCiHook(BaseHook): form = JenkinsForm db_object = PagureCI backref = 'hook_pagure_ci' - form_fields = [ - 'pagure_name', 'jenkins_name', - 'jenkins_url', 'jenkins_token', 'active' - ] + form_fields = ['jenkins_name', 'jenkins_url', 'jenkins_token', 'active'] @classmethod def set_up(cls, project): diff --git a/pagure/lib/pagure_ci.py b/pagure/lib/pagure_ci.py index 88ddcd7..c6b9ee7 100644 --- a/pagure/lib/pagure_ci.py +++ b/pagure/lib/pagure_ci.py @@ -36,7 +36,7 @@ def process_pr(logger, cfg, pr_id, repo, branch): 'REPO': repo, 'BRANCH': branch}) else: - raise pagure.exceptions.HookInactiveException(cfg.pagure_name) + raise pagure.exceptions.HookInactiveException(cfg.project.name) def process_build(logger, cfg, build_id): @@ -63,11 +63,11 @@ def process_build(logger, cfg, build_id): return # Comment in Pagure - logger.info('Updating %s PR %d: %s', cfg.pagure_name, pr_id, result) + logger.info('Updating %s PR %d: %s', cfg.project.name, pr_id, result) try: pagure_ci_flag(logger, username=cfg.display_name, - repo=cfg.pagure_name, + repo=cfg.project.name, requestid=pr_id, result=result, url=url) @@ -75,7 +75,7 @@ def process_build(logger, cfg, build_id): except KeyError as exc: logger.warning('Unknown build status', exc_info=exc) else: - raise pagure.exceptions.HookInactiveException(cfg.pagure_name) + raise pagure.exceptions.HookInactiveException(cfg.project.name) def post_data(logger, *args, **kwargs): diff --git a/pagure/ui/plugins.py b/pagure/ui/plugins.py index d7b0e56..1801ef3 100644 --- a/pagure/ui/plugins.py +++ b/pagure/ui/plugins.py @@ -19,7 +19,7 @@ import pagure.lib import pagure.forms from pagure import APP, SESSION, login_required, is_repo_admin from pagure.lib.model import BASE -from pagure.exceptions import FileNotFoundException +from pagure.exceptions import FileNotFoundException, ConfigNotFound from pagure.hooks import jenkins_hook from pagure.lib import model @@ -189,15 +189,15 @@ if pagure_ci is not None: try: data = json.loads(flask.request.get_data()) - cfg = jenkins_hook.get_configs( - data['name'], jenkins_hook.Service.JENKINS)[0] + service_obj = jenkins_hook.Service() + cfg = jenkins_hook.get_configs(data['name'], service_obj.JENKINS)[0] build_id = data['build']['number'] if not constant_time.bytes_eq( to_bytes(pagure_ci_token), to_bytes(cfg.pagure_ci_token)): return ('Token mismatch', 401) - except (TypeError, ValueError, KeyError, jenkins_hook.ConfigNotFound) as exc: + except (TypeError, ValueError, KeyError, ConfigNotFound) as exc: APP.logger.error('Error processing jenkins notification', exc_info=exc) flask.abort(400, "Bad Request") diff --git a/pagureCI/consumer.py b/pagureCI/consumer.py index ea97f30..1eb1113 100644 --- a/pagureCI/consumer.py +++ b/pagureCI/consumer.py @@ -39,7 +39,7 @@ class Integrator(fedmsg.consumers.FedmsgConsumer): self.trigger_build(msg) else: self.process_build(msg) - except jenkins_hook.ConfigNotFound as exc: + except pagure.exceptions.ConfigNotFound as exc: self.log.info('Unconfigured project %r', str(exc)) except pagure.exceptions.HookInactiveException as exc: self.log.info('Hook Inactive for project %r', str(exc)) @@ -49,8 +49,9 @@ class Integrator(fedmsg.consumers.FedmsgConsumer): pr_id = msg['pullrequest']['id'] project = msg['pullrequest']['project']['name'] branch = msg['pullrequest']['branch_from'] + service_obj = jenkins_hook.Service() - for cfg in jenkins_hook.get_configs(project, jenkins_hook.Service.PAGURE): + for cfg in jenkins_hook.get_configs(project, service_obj.PAGURE): repo = msg['pullrequest'].get('remote_git') or get_repo(cfg, msg) self.log.info("Trigger on %s PR #%s from %s: %s", project, pr_id, repo, branch) @@ -59,7 +60,7 @@ class Integrator(fedmsg.consumers.FedmsgConsumer): def process_build(self, msg): ''' Extracts the information from the build and flag the pull-request. ''' - for cfg in jenkins_hook.get_configs(msg['project'], jenkins_hook.Service.JENKINS): + for cfg in jenkins_hook.get_configs(msg['project'], service_obj.JENKINS): pagure_ci.process_build(self.log, cfg, msg['build']) diff --git a/tests/test_pagure_flask_ui_plugins_pagure_ci.py b/tests/test_pagure_flask_ui_plugins_pagure_ci.py index 5db71c1..4ea15f3 100644 --- a/tests/test_pagure_flask_ui_plugins_pagure_ci.py +++ b/tests/test_pagure_flask_ui_plugins_pagure_ci.py @@ -57,9 +57,6 @@ class PagureFlaskPluginPagureCItests(tests.Modeltests): 'test project #1 ', output.data) self.assertTrue('

Pagure CI settings

' in output.data) self.assertTrue( - '' - in output.data) - self.assertTrue( '' in output.data) self.assertTrue( @@ -84,9 +81,6 @@ class PagureFlaskPluginPagureCItests(tests.Modeltests): 'test project #1 ', output.data) self.assertTrue('

Pagure CI settings

' in output.data) self.assertTrue( - '' - in output.data) - self.assertTrue( '' in output.data) self.assertTrue( @@ -103,7 +97,6 @@ class PagureFlaskPluginPagureCItests(tests.Modeltests): data = { 'csrf_token': csrf_token, 'active': 'y', - 'pagure_name': 'test', 'jenkins_name': 'jenkins_test', 'jenkins_url': 'https://jenkins.fedoraproject.org', 'jenkins_token': 'BEEFCAFE' @@ -128,7 +121,7 @@ class PagureFlaskPluginPagureCItests(tests.Modeltests): self.assertFalse( '\n Hook activated' in output.data) self.assertTrue( - '' + '' '\nThis field is required.' in output.data) self.assertTrue( @@ -141,9 +134,6 @@ class PagureFlaskPluginPagureCItests(tests.Modeltests): 'test project #1 ', output.data) self.assertTrue('

Pagure CI settings

' in output.data) self.assertTrue( - '' - in output.data) - self.assertTrue( '' in output.data) self.assertTrue( @@ -169,7 +159,7 @@ class PagureFlaskPluginPagureCItests(tests.Modeltests): self.assertFalse( '\n Hook activated' in output.data) self.assertTrue( - '' + '' '\nThis field is required.' in output.data) self.assertTrue( @@ -180,7 +170,6 @@ class PagureFlaskPluginPagureCItests(tests.Modeltests): data = { 'csrf_token': csrf_token, 'active': 'y', - 'pagure_name': 'test', 'jenkins_name': 'jenkins_test', 'jenkins_url': 'https://jenkins.fedoraproject.org', 'jenkins_token': 'BEEFCAFE' @@ -201,12 +190,6 @@ class PagureFlaskPluginPagureCItests(tests.Modeltests): 'test project #1 ', output.data) self.assertTrue('

Pagure CI settings

' in output.data) self.assertTrue( - '' - in output.data) - self.assertTrue( - '' - in output.data) - self.assertTrue( '' in output.data) self.assertTrue( @@ -219,7 +202,6 @@ class PagureFlaskPluginPagureCItests(tests.Modeltests): # De-Activate hook data = { 'csrf_token': csrf_token, - 'pagure_name': 'test', 'jenkins_name': 'jenkins_test', 'jenkins_url': 'https://jenkins.fedoraproject.org', 'jenkins_token': 'BEEFCAFE'