-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Retrigger downstream koji builds #1586
Merged
softwarefactory-project-zuul
merged 1 commit into
packit:main
from
majamassarini:fix/packit-service/1540
Jul 26, 2022
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,8 @@ | |
from datetime import datetime | ||
from typing import Optional, Dict | ||
|
||
from fasjson_client import Client | ||
from fasjson_client.errors import APIError | ||
from ogr.abstract import PullRequest, PRStatus | ||
|
||
from packit.api import PackitAPI | ||
|
@@ -23,6 +25,7 @@ | |
from packit_service.config import PackageConfigGetter, ProjectToSync | ||
from packit_service.constants import ( | ||
CONTACTS_URL, | ||
FASJSON_URL, | ||
FILE_DOWNLOAD_FAILURE, | ||
MSG_RETRIGGER, | ||
) | ||
|
@@ -39,6 +42,7 @@ | |
ReleaseEvent, | ||
AbstractIssueCommentEvent, | ||
CheckRerunReleaseEvent, | ||
PullRequestCommentPagureEvent, | ||
) | ||
from packit_service.worker.handlers.abstract import ( | ||
JobHandler, | ||
|
@@ -380,7 +384,9 @@ def run(self) -> TaskResults: | |
|
||
|
||
@configured_as(job_type=JobType.koji_build) | ||
@run_for_comment(command="koji-build") | ||
@reacts_to(event=PushPagureEvent) | ||
@reacts_to(event=PullRequestCommentPagureEvent) | ||
class DownstreamKojiBuildHandler(RetriableJobHandler): | ||
""" | ||
This handler can submit a build in Koji from a dist-git. | ||
|
@@ -404,6 +410,7 @@ def __init__( | |
) | ||
self.dg_branch = event.get("git_ref") | ||
self._pull_request: Optional[PullRequest] = None | ||
self._packit_api = None | ||
|
||
@property | ||
def pull_request(self): | ||
|
@@ -421,10 +428,38 @@ def pull_request(self): | |
self._pull_request = prs[0] | ||
return self._pull_request | ||
|
||
@property | ||
def packit_api(self): | ||
if not self._packit_api: | ||
self._packit_api = PackitAPI( | ||
self.service_config, | ||
self.job_config, | ||
downstream_local_project=self.local_project, | ||
) | ||
return self._packit_api | ||
|
||
def get_pr_author(self): | ||
"""Get the login of the author of the PR (if there is any corresponding PR).""" | ||
return self.pull_request.author if self.pull_request else None | ||
|
||
def is_packager(self, user): | ||
"""Check that the given FAS user | ||
is a packager | ||
|
||
Args: | ||
user (str) FAS user account name | ||
Returns: | ||
true if a packager false otherwise | ||
""" | ||
self.packit_api.init_kerberos_ticket() | ||
client = Client(FASJSON_URL) | ||
try: | ||
groups = client.list_user_groups(username=user) | ||
except APIError: | ||
logger.debug(f"Unable to get groups for user {user}.") | ||
return False | ||
return "packager" in [group["groupname"] for group in groups.result] | ||
|
||
def pre_check(self) -> bool: | ||
if self.data.event_type in (PushPagureEvent.__name__,): | ||
if self.data.git_ref not in ( | ||
|
@@ -460,6 +495,17 @@ def pre_check(self) -> bool: | |
f"configuration: {self.job_config.allowed_committers}." | ||
) | ||
return False | ||
elif self.data.event_type in (PullRequestCommentPagureEvent.__name__,): | ||
commenter = self.data.actor | ||
logger.debug( | ||
f"Triggering downstream koji build through comment by: {commenter}" | ||
) | ||
if not self.is_packager(commenter): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in this case, it could be nice to notify user about this, but we can discuss if and how we want to do it (PR comment?) as a follow-up and be also consistent with retriggering bodhi updates |
||
logger.info( | ||
f"koji-build retrigger comment event on PR identifier {self.data.pr_id} " | ||
f"done by {commenter} which is not a packager." | ||
) | ||
return False | ||
|
||
return True | ||
|
||
|
@@ -474,14 +520,14 @@ def run(self) -> TaskResults: | |
if self.service_config.repository_cache | ||
else None, | ||
) | ||
packit_api = PackitAPI( | ||
self.service_config, | ||
self.job_config, | ||
downstream_local_project=self.local_project, | ||
branch = ( | ||
self.project.get_pr(self.data.pr_id).target_branch | ||
if self.data.event_type in (PullRequestCommentPagureEvent.__name__,) | ||
else self.dg_branch | ||
) | ||
try: | ||
packit_api.build( | ||
dist_git_branch=self.dg_branch, | ||
self.packit_api.build( | ||
dist_git_branch=branch, | ||
scratch=self.job_config.scratch, | ||
nowait=True, | ||
from_upstream=False, | ||
|
@@ -509,12 +555,7 @@ def run(self) -> TaskResults: | |
issue_repo = self.service_config.get_project( | ||
url=self.job_config.issue_repository | ||
) | ||
body = ( | ||
f"Koji build on `{self.dg_branch}` branch failed:\n" | ||
"```\n" | ||
f"{ex}\n" | ||
"```" | ||
) | ||
body = f"Koji build on `{branch}` branch failed:\n" "```\n" f"{ex}\n" "```" | ||
PackageConfigGetter.create_issue_if_needed( | ||
project=issue_repo, | ||
title="Fedora Koji build failed to be triggered", | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,127 @@ | ||
{ | ||
"agent": "mmassari", | ||
"pullrequest": { | ||
"assignee": null, | ||
"branch": "test-koji-downstream", | ||
"branch_from": "test-koji-downstream-2", | ||
"cached_merge_status": "FFORWARD", | ||
"closed_at": null, | ||
"closed_by": null, | ||
"comments": [ | ||
{ | ||
"comment": "/packit koji-build", | ||
"commit": null, | ||
"date_created": "1657783448", | ||
"edited_on": null, | ||
"editor": null, | ||
"filename": null, | ||
"id": 110401, | ||
"line": null, | ||
"notification": false, | ||
"parent": null, | ||
"reactions": {}, | ||
"tree": null, | ||
"user": { | ||
"full_url": "https://src.fedoraproject.org/user/mmassari", | ||
"fullname": "Maja Massarini", | ||
"name": "mmassari", | ||
"url_path": "user/mmassari" | ||
} | ||
} | ||
], | ||
"commit_start": "beaf90bcecc51968a46663f8d6f092bfdc92e682", | ||
"commit_stop": "beaf90bcecc51968a46663f8d6f092bfdc92e682", | ||
"date_created": "1657783415", | ||
"full_url": "https://src.fedoraproject.org/rpms/python-teamcity-messages/pull-request/36", | ||
"id": 36, | ||
"initial_comment": null, | ||
"last_updated": "1657783448", | ||
"project": { | ||
"access_groups": { | ||
"admin": [], | ||
"collaborator": [], | ||
"commit": [], | ||
"ticket": [] | ||
}, | ||
"access_users": { | ||
"admin": ["limb"], | ||
"collaborator": [], | ||
"commit": [], | ||
"owner": ["mmassari"], | ||
"ticket": [] | ||
}, | ||
"close_status": [], | ||
"custom_keys": [], | ||
"date_created": "1643654065", | ||
"date_modified": "1650542166", | ||
"description": "The python-teamcity-messages package\\n", | ||
"full_url": "https://src.fedoraproject.org/rpms/python-teamcity-messages", | ||
"fullname": "rpms/python-teamcity-messages", | ||
"id": 54766, | ||
"milestones": {}, | ||
"name": "python-teamcity-messages", | ||
"namespace": "rpms", | ||
"parent": null, | ||
"priorities": {}, | ||
"tags": [], | ||
"url_path": "rpms/python-teamcity-messages", | ||
"user": { | ||
"full_url": "https://src.fedoraproject.org/user/mmassari", | ||
"fullname": "Maja Massarini", | ||
"name": "mmassari", | ||
"url_path": "user/mmassari" | ||
} | ||
}, | ||
"remote_git": null, | ||
"repo_from": { | ||
"access_groups": { | ||
"admin": [], | ||
"collaborator": [], | ||
"commit": [], | ||
"ticket": [] | ||
}, | ||
"access_users": { | ||
"admin": ["limb"], | ||
"collaborator": [], | ||
"commit": [], | ||
"owner": ["mmassari"], | ||
"ticket": [] | ||
}, | ||
"close_status": [], | ||
"custom_keys": [], | ||
"date_created": "1643654065", | ||
"date_modified": "1650542166", | ||
"description": "The python-teamcity-messages package\\n", | ||
"full_url": "https://src.fedoraproject.org/rpms/python-teamcity-messages", | ||
"fullname": "rpms/python-teamcity-messages", | ||
"id": 54766, | ||
"milestones": {}, | ||
"name": "python-teamcity-messages", | ||
"namespace": "rpms", | ||
"parent": null, | ||
"priorities": {}, | ||
"tags": [], | ||
"url_path": "rpms/python-teamcity-messages", | ||
"user": { | ||
"full_url": "https://src.fedoraproject.org/user/mmassari", | ||
"fullname": "Maja Massarini", | ||
"name": "mmassari", | ||
"url_path": "user/mmassari" | ||
} | ||
}, | ||
"status": "Open", | ||
"tags": [], | ||
"threshold_reached": null, | ||
"title": "Not usefull commit", | ||
"uid": "d8b1dd625c674cb9ad8010985bd98351", | ||
"updated_on": "1657783448", | ||
"user": { | ||
"full_url": "https://src.fedoraproject.org/user/mmassari", | ||
"fullname": "Maja Massarini", | ||
"name": "mmassari", | ||
"url_path": "user/mmassari" | ||
} | ||
}, | ||
"topic": "org.fedoraproject.prod.pagure.pull-request.comment.added", | ||
"timestamp": 1657776351.055628 | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see that we are doing similar thing here (I am implementing #1541). Can you please move this
elif
block to the method intoJobHandler
superclass so I can later use it inCreateBodhiUpdateHandler
? Just moving this block of code to separated method in the superclass is enough... I will update this method later to fit the needs of both cases.(Or if you want, replacing the
koji builds
part in the stings with some variable should be enough)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I have fixed it, as Laura wrote maybe I am doing the wrong check now, I will move it in a separate method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved the check in a separate method, but in the same class. I am not really sure you should reuse this check. I think that not every packager (as is for the
koji-build
) should be able to do abodhi-update
but I may be wrong.