PR#1165: Fix manually entering pagure project name for Pagure CI - pagure

pagure

#1165 Fix manually entering pagure project name for Pagure CI

Closed 7 years ago by farhaan. Opened 7 years ago by farhaan.

farhaan/pagure jenkins-hook into master

Fix manually entering pagure project name for Pagure CI

Farhaan Bukhsh • 7 years ago

a6a45fb

pagure/exceptions.py

file modified

		`@@ -62,3 +62,8 @@`
		`class HookInactiveException(PagureException):`
		`''' Exception raised when the hook is inactive. '''`
		`pass`
		`+`
		`+`
		`+ class ConfigNotFound(PagureException):`
		`+ ''' Exception raised when configurations are not found in database. '''`
		`+ pass`

pagure/hooks/jenkins_hook.py

file modified

+14 -21

		`@@ -16,10 +16,11 @@`
		`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 @@`

		`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 @@`
		`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):`
farhaan commented 7 years ago @pingou sorry I rebased. I guess I can't directly use PagureCI.project. Since I need to have an object of PagureCI for which I need to pass. I tried it though but may be I don't know how to use it and hence I opted for this method.
		`+ 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 @@`


		`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 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 @@`
		`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):`

pagure/lib/pagure_ci.py

file modified

+4 -4

		`@@ -36,7 +36,7 @@`
		`'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 @@`
		`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 @@`
		`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):`

pagure/ui/plugins.py

file modified

+4 -4

		`@@ -19,7 +19,7 @@`
		`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 @@`

		`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")`

pagureCI/consumer.py

file modified

+4 -3

		`@@ -39,7 +39,7 @@`
		`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 @@`
		`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 @@`

		`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'])`

tests/test_pagure_flask_ui_plugins_pagure_ci.py

file modified

+2 -20

		`@@ -57,9 +57,6 @@`
		`'test project #1 </div>', output.data)`
		`self.assertTrue('<h3>Pagure CI settings</h3>' in output.data)`
		`self.assertTrue(`
		`- '<td><label for="pagure_name">Name of project in Pagure</label></td>'`
		`- in output.data)`
		`- self.assertTrue(`
		`'<td><label for="jenkins_name">Name of project in Jenkins</label></td>'`
		`in output.data)`
		`self.assertTrue(`
		`@@ -84,9 +81,6 @@`
		`'test project #1 </div>', output.data)`
		`self.assertTrue('<h3>Pagure CI settings</h3>' in output.data)`
		`self.assertTrue(`
		`- '<td><label for="pagure_name">Name of project in Pagure</label></td>'`
		`- in output.data)`
		`- self.assertTrue(`
		`'<td><label for="jenkins_name">Name of project in Jenkins</label></td>'`
		`in output.data)`
		`self.assertTrue(`
		`@@ -103,7 +97,6 @@`
		`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 @@`
		`self.assertFalse(`
		`'</button>\n Hook activated' in output.data)`
		`self.assertTrue(`
		`- '<td><input id="pagure_name" name="pagure_name" type="text" value=""></td>'`
		`+ '<td><input id="jenkins_name" name="jenkins_name" type="text" value=""></td>'`
		`'\n<td class="errors">This field is required.</td>'`
		`in output.data)`
		`self.assertTrue(`
		`@@ -141,9 +134,6 @@`
		`'test project #1 </div>', output.data)`
		`self.assertTrue('<h3>Pagure CI settings</h3>' in output.data)`
		`self.assertTrue(`
		`- '<td><label for="pagure_name">Name of project in Pagure</label></td>'`
		`- in output.data)`
		`- self.assertTrue(`
		`'<td><label for="jenkins_name">Name of project in Jenkins</label></td>'`
		`in output.data)`
		`self.assertTrue(`
		`@@ -169,7 +159,7 @@`
		`self.assertFalse(`
		`'</button>\n Hook activated' in output.data)`
		`self.assertTrue(`
		`- '<td><input id="pagure_name" name="pagure_name" type="text" value=""></td>'`
		`+ '<td><input id="jenkins_name" name="jenkins_name" type="text" value=""></td>'`
		`'\n<td class="errors">This field is required.</td>'`
		`in output.data)`
		`self.assertTrue(`
		`@@ -180,7 +170,6 @@`
		`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 @@`
		`'test project #1 </div>', output.data)`
		`self.assertTrue('<h3>Pagure CI settings</h3>' in output.data)`
		`self.assertTrue(`
		`- '<td><label for="pagure_name">Name of project in Pagure</label></td>'`
		`- in output.data)`
		`- self.assertTrue(`
		`- '<td><input id="pagure_name" name="pagure_name" type="text" value="test"></td>'`
		`- in output.data)`
		`- self.assertTrue(`
		`'<td><label for="jenkins_name">Name of project in Jenkins</label></td>'`
		`in output.data)`
		`self.assertTrue(`
		`@@ -219,7 +202,6 @@`
		`# De-Activate hook`
		`data = {`
		`'csrf_token': csrf_token,`
		`- 'pagure_name': 'test',`
		`'jenkins_name': 'jenkins_test',`
		`'jenkins_url': 'https://jenkins.fedoraproject.org',`
		`'jenkins_token': 'BEEFCAFE'`

farhaan commented 7 years ago

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.

pingou commented on line 38 of pagure/hooks/jenkins_hook.py 7 years ago

Why aren't you just using PagureCI.project ?

rebased

7 years ago

rebased

7 years ago

farhaan commented on line 36 of pagure/hooks/jenkins_hook.py 7 years ago

@pingou sorry I rebased. I guess I can't directly use PagureCI.project. Since I need to have an object of PagureCI for which I need to pass. I tried it though but may be I don't know how to use it and hence I opted for this method.

farhaan commented 7 years ago

I think with the new pagure Ci code this Pr doesn't make sense :smile: should I close it ?

pingou commented 7 years ago

Sounds good, yes, let's close this one :)

Pull-Request has been closed by farhaan

7 years ago