-
Notifications
You must be signed in to change notification settings - Fork 135
Emoji approve based on changes and code owners #254
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,8 @@ | ||
import logging as log | ||
import fnmatch | ||
import re | ||
import shlex | ||
|
||
from . import gitlab | ||
|
||
GET, POST, PUT = gitlab.GET, gitlab.POST, gitlab.PUT | ||
|
@@ -8,16 +13,128 @@ class Approvals(gitlab.Resource): | |
|
||
def refetch_info(self): | ||
gitlab_version = self._api.version() | ||
if gitlab_version.release >= (9, 2, 2): | ||
approver_url = '/projects/{0.project_id}/merge_requests/{0.iid}/approvals'.format(self) | ||
else: | ||
# GitLab botched the v4 api before 9.2.3 | ||
approver_url = '/projects/{0.project_id}/merge_requests/{0.id}/approvals'.format(self) | ||
|
||
if gitlab_version.is_ee: | ||
self._info = self._api.call(GET(approver_url)) | ||
self._info = self._api.call(GET(self.get_approvals_url(self))) | ||
else: | ||
self._info = dict(self._info, approvals_left=0, approved_by=[]) | ||
self._info = self.get_approvers_ce() | ||
|
||
def get_approvers_ce(self): | ||
"""get approvers status using thumbs on merge request | ||
""" | ||
gitlab_version = self._api.version() | ||
|
||
# Gitlab supports approvals in free version after 13.2.0 | ||
approval_api_results = dict(approved_by=[]) | ||
if gitlab_version.release >= (13, 2, 0): | ||
approval_api_results = self._api.call(GET(self.get_approvals_url(self))) | ||
|
||
owner_file = self.get_codeowners_ce() | ||
if not owner_file['owners']: | ||
log.info("No CODEOWNERS file in master, continuing without approvers flow") | ||
return dict(self._info, approvals_left=0, approved_by=[], codeowners=[]) | ||
|
||
code_owners = self.determine_responsible_owners(owner_file['owners'], self.get_changes_ce()) | ||
|
||
if not code_owners: | ||
log.info("No matched code owners, continuing without approvers flow") | ||
return dict(self._info, approvals_left=0, approved_by=[], codeowners=[]) | ||
|
||
awards = self.get_awards_ce() | ||
|
||
up_votes = [e for e in awards if e['name'] == 'thumbsup' and e['user']['username'] in code_owners] | ||
for approver in approval_api_results['approved_by']: | ||
if approver['user']['username'] in code_owners \ | ||
and not any(ex['user']['username'] == approver['user']['username'] for ex in up_votes): | ||
up_votes.append(approver) | ||
|
||
approvals_required = len(code_owners) | ||
|
||
if owner_file['approvals_required'] > 0: | ||
approvals_required = owner_file['approvals_required'] | ||
|
||
approvals_left = max(approvals_required - len(up_votes), 0) | ||
|
||
return dict(self._info, approvals_left=approvals_left, approved_by=up_votes, codeowners=code_owners) | ||
|
||
def determine_responsible_owners(self, owners_glob, changes): | ||
owners = set([]) | ||
|
||
# Always add global users | ||
if '*' in owners_glob: | ||
owners.update(owners_glob['*']) | ||
|
||
if 'changes' not in changes: | ||
log.info("No changes in merge request!?") | ||
return owners | ||
|
||
for change in changes['changes']: | ||
for glob, users in owners_glob.items(): | ||
test_glob = glob | ||
if glob.endswith('/'): | ||
test_glob += "*" | ||
|
||
if 'new_path' in change and fnmatch.fnmatch(change['new_path'], test_glob): | ||
owners.update(users) | ||
|
||
return owners | ||
|
||
def get_changes_ce(self): | ||
changes_url = '/projects/{0.project_id}/merge_requests/{0.iid}/changes' | ||
changes_url = changes_url.format(self) | ||
|
||
return self._api.call(GET(changes_url)) | ||
|
||
def get_awards_ce(self): | ||
emoji_url = '/projects/{0.project_id}/merge_requests/{0.iid}/award_emoji' | ||
emoji_url = emoji_url.format(self) | ||
return self._api.call(GET(emoji_url)) | ||
|
||
def get_codeowners_ce(self): | ||
config_file = self._api.repo_file_get(self.project_id, "CODEOWNERS", "master") | ||
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. This should point to the project's 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. I agree. I will have a look at this |
||
owner_globs = {} | ||
required = 0 | ||
required_regex = re.compile('.*MARGEBOT_MINIMUM_APPROVERS *= *([0-9]+)') | ||
|
||
if config_file is None: | ||
return {"approvals_required": 0, "owners": {}} | ||
|
||
for line in config_file['content'].splitlines(): | ||
if 'MARGEBOT_' in line: | ||
match = required_regex.match(line) | ||
if match: | ||
required = int(match.group(1)) | ||
|
||
if line != "" and not line.startswith(' ') and not line.startswith('#'): | ||
elements = shlex.split(line) | ||
glob = elements.pop(0) | ||
owner_globs.setdefault(glob, set([])) | ||
|
||
for user in elements: | ||
owner_globs[glob].add(user.strip('@')) | ||
Comment on lines
+102
to
+114
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. The matching order for patterns should be ordered and reversed, see https://docs.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners (gitlab has the same):
Maybe use https://github.com/sbdchd/codeowners for this instead and any more issues can be solved upstream? 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. Thanks for the suggestion. I will have a look at the library |
||
|
||
return {"approvals_required": required, "owners": owner_globs} | ||
|
||
@property | ||
def approvers_string(self): | ||
reviewer_string = "" | ||
|
||
if len(self.codeowners) == 1: | ||
reviewer_string = '@' + self.codeowners.pop() | ||
elif len(self.codeowners) > 1: | ||
reviewer_ats = ["@" + reviewer for reviewer in self.codeowners] | ||
reviewer_string = '{} or {}'.format(', '.join(reviewer_ats[:-1]), reviewer_ats[-1]) | ||
|
||
return reviewer_string | ||
|
||
def get_approvals_url(self, obj): | ||
gitlab_version = self._api.version() | ||
|
||
if gitlab_version.release >= (9, 2, 2): | ||
return '/projects/{0.project_id}/merge_requests/{0.iid}/approvals'.format(obj) | ||
|
||
# GitLab botched the v4 api before 9.2.3 | ||
return '/projects/{0.project_id}/merge_requests/{0.id}/approvals'.format(obj) | ||
|
||
@property | ||
def iid(self): | ||
|
@@ -44,6 +161,14 @@ def approver_ids(self): | |
"""Return the uids of the approvers.""" | ||
return [who['user']['id'] for who in self.info['approved_by']] | ||
|
||
@property | ||
def codeowners(self): | ||
"""Only used for gitlab CE""" | ||
if 'codeowners' in self.info: | ||
return self.info['codeowners'] | ||
|
||
return {} | ||
|
||
def reapprove(self): | ||
"""Impersonates the approvers and re-approves the merge_request as them. | ||
|
||
|
@@ -55,11 +180,9 @@ def reapprove(self): | |
|
||
def approve(self, obj): | ||
"""Approve an object which can be a merge_request or an approval.""" | ||
if self._api.version().release >= (9, 2, 2): | ||
approve_url = '/projects/{0.project_id}/merge_requests/{0.iid}/approve'.format(obj) | ||
else: | ||
# GitLab botched the v4 api before 9.2.3 | ||
approve_url = '/projects/{0.project_id}/merge_requests/{0.id}/approve'.format(obj) | ||
gitlab_version = self._api.version() | ||
|
||
for uid in self.approver_ids: | ||
self._api.call(POST(approve_url), sudo=uid) | ||
# Gitlab supports approvals in free version after 13.2.0 | ||
if gitlab_version.is_ee or gitlab_version.release >= (13, 2, 0): | ||
for uid in self.approver_ids: | ||
self._api.call(POST(self.get_approvals_url(obj)), sudo=uid) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import json | ||
import logging as log | ||
import urllib.parse | ||
from collections import namedtuple | ||
|
||
import requests | ||
|
@@ -10,7 +11,7 @@ def __init__(self, gitlab_url, auth_token): | |
self._auth_token = auth_token | ||
self._api_base_url = gitlab_url.rstrip('/') + '/api/v4' | ||
|
||
def call(self, command, sudo=None): | ||
def call_raw(self, command, sudo=None): | ||
method = command.method | ||
url = self._api_base_url + command.endpoint | ||
headers = {'PRIVATE-TOKEN': self._auth_token} | ||
|
@@ -35,7 +36,7 @@ def call(self, command, sudo=None): | |
return True # NoContent | ||
|
||
if response.status_code < 300: | ||
return command.extract(response.json()) if command.extract else response.json() | ||
return response | ||
|
||
if response.status_code == 304: | ||
return False # Not Modified | ||
|
@@ -64,6 +65,40 @@ def other_error(code, msg): | |
|
||
raise error(response.status_code, err_message) | ||
|
||
def call(self, command, sudo=None): | ||
"""call gitlab api and parse response json""" | ||
response = self.call_raw(command, sudo) | ||
return command.extract(response.json()) if command.extract else response.json() | ||
|
||
def repo_file_get(self, project_id, file_path, ref): | ||
"""Get file from repository | ||
|
||
The function returns a dict similar to the JSON except | ||
that "encoding" is missing as the content is already | ||
provided decoded as bytes. | ||
|
||
See: https://docs.gitlab.com/ce/api/repository_files.html#get-file-from-repository | ||
""" | ||
file_path = urllib.parse.quote(file_path, safe="") | ||
url = '/projects/{id}/repository/files/{file_path}/raw?ref={ref}' | ||
url = url.format(id=project_id, file_path=file_path, ref=ref) | ||
|
||
try: | ||
result = self.call_raw(GET(url)) | ||
return { | ||
'file_name': result.headers['X-Gitlab-File-Name'], | ||
'file_path': result.headers['X-Gitlab-File-Path'], | ||
'size': result.headers['X-Gitlab-Size'], | ||
'content_sha256': result.headers['X-Gitlab-Content-Sha256'], | ||
'blob_id': result.headers['X-Gitlab-Blob-Id'], | ||
'commit_id': result.headers['X-Gitlab-Commit-Id'], | ||
'last_commit_id': result.headers['X-Gitlab-Last-Commit-Id'], | ||
'content': result.text, | ||
'ref': result.headers['X-Gitlab-Ref'], | ||
} | ||
except NotFound: | ||
return None | ||
|
||
Comment on lines
+68
to
+101
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. I would say use https://github.com/python-gitlab/python-gitlab/ for this (disclaimer, I help maintain it so I'm biased here) but I realize the rest of this project doesn't use it and it would take more refactoring for that. But this would be a one-liner if you used a library. 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. It wouldn't make sense to introduce a new library for this PR, however, it looks like a great idea of another PR 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. Yeah, you're right, this just needs to get merged :) |
||
def collect_all_pages(self, get_command): | ||
result = [] | ||
fetch_again, page_no = True, 1 | ||
|
Uh oh!
There was an error while loading. Please reload this page.