From c24fab9d73c4a7d782051687b8c9ea494935a328 Mon Sep 17 00:00:00 2001 From: Jeremy Stanley Date: Thu, 29 Aug 2019 17:05:09 +0000 Subject: [PATCH] Include a -v/--verbose option for check commands So as to increase transparency of the candidate verification checks, add a --verbose option to all of the various candidate checking command scripts. Supplying it once returns check result URLs for clarity, while adding it twice also displays the query URLs used. Set double verbosity for the check review tox testenv, to aid election officials in reviewing nominees for valid candidacy. Also update the Gerrit and Git URLs for the OpenDev transition and correct a couple of docstrings for utility functions. Change-Id: I5f6fa4e2c2c6058ba5090078bbdf9dd9f31f692e --- openstack_election/check_candidacy.py | 19 +++-- .../cmds/check_all_candidacies.py | 22 ++--- openstack_election/cmds/check_candidacy.py | 4 +- openstack_election/cmds/check_manual.py | 5 +- .../cmds/ci_check_all_candidate_files.py | 18 +++-- openstack_election/cmds/template_emails.py | 2 +- openstack_election/owners.py | 12 ++- .../templates/tc_campaigning_kickoff.j2 | 2 +- openstack_election/utils.py | 81 ++++++++++--------- tox.ini | 2 +- 10 files changed, 94 insertions(+), 73 deletions(-) diff --git a/openstack_election/check_candidacy.py b/openstack_election/check_candidacy.py index 88d3feed..2c507e74 100755 --- a/openstack_election/check_candidacy.py +++ b/openstack_election/check_candidacy.py @@ -23,7 +23,7 @@ from openstack_election import utils # FIXME: Printing from library function isn't great. # change API to return the messages and let the consumer decide what to # do with them -def check_candidate(project_name, email, projects, limit=1): +def check_candidate(project_name, email, projects, limit=1, verbose=0): def pretty_datetime(dt_str): dt = datetime.datetime.strptime(dt_str.split('.')[0], '%Y-%m-%d %H:%M:%S') @@ -57,15 +57,14 @@ def check_candidate(project_name, email, projects, limit=1): owner, repo_name)) if branch: query += (' branch:%s' % (branch)) - print('Checking %s for merged changes by %s' % - (repo_name, email)) - for review in utils.get_reviews(query): - url = ('%s/%s/commit/?id=%s' % ( - utils.CGIT_URL, review['project'], - review['current_revision'])) - print('%2d: %s %s' % - (found, pretty_datetime(review['submitted']), - url)) + if verbose >= 1: + print('Checking %s for merged changes by %s' % + (repo_name, email)) + for review in utils.get_reviews(query, verbose=verbose): + print('Found: %s/%s merged on %s to %s for %s' % ( + utils.GERRIT_BASE, review['_number'], + pretty_datetime(review['submitted']), repo_name, + project_name)) found += 1 if found >= limit: return found diff --git a/openstack_election/cmds/check_all_candidacies.py b/openstack_election/cmds/check_all_candidacies.py index 42a58976..671cad6e 100755 --- a/openstack_election/cmds/check_all_candidacies.py +++ b/openstack_election/cmds/check_all_candidacies.py @@ -24,15 +24,15 @@ from six.moves import input results = [] -def get_reviews(): +def get_reviews(verbose=0): return utils.get_reviews('is:open project:%s file:^%s/%s/.*' % (utils.ELECTION_REPO, utils.CANDIDATE_PATH, - utils.conf['release'])) + utils.conf['release']), verbose=verbose) -def print_member(filepath): +def print_member(filepath, verbose=0): email = utils.get_email(filepath) - member = utils.lookup_member(email) + member = utils.lookup_member(email, verbose=verbose) member_id = member.get('data', [{}])[0].get('id') base = 'https://www.openstack.org/community/members/profile' print('OSF member profile: %s/%s' % (base, member_id)) @@ -52,12 +52,14 @@ def main(): action='store_true', help=('Pause after each review to manually post ' 'results')) + parser.add_argument('-v', '--verbose', action="count", default=0, + help='Increase program verbosity') args = parser.parse_args() projects = utils.get_projects(tag=args.tag, fallback_to_master=True) election_type = utils.conf.get('election_type', '').lower() - for review in get_reviews(): + for review in get_reviews(verbose=args.verbose): if review['status'] != 'NEW': continue @@ -75,7 +77,8 @@ def main(): candiate_ok = checks.validate_filename(filepath) if candiate_ok: - candiate_ok = checks.validate_member(filepath) + candiate_ok = checks.validate_member(filepath, + verbose=args.verbose) if candiate_ok: # If we're a PTL election OR if the team is not TC we need @@ -85,9 +88,10 @@ def main(): if args.interactive: print('The following commit and profile validate this ' 'candidate:') - candiate_ok = checks.check_for_changes(projects, filepath, - args.limit) - print_member(filepath) + candiate_ok = checks.check_for_changes( + projects, filepath, args.limit, + verbose=args.verbose) + print_member(filepath, verbose=args.verbose) else: print('Not checking for changes as this is a TC election') else: diff --git a/openstack_election/cmds/check_candidacy.py b/openstack_election/cmds/check_candidacy.py index a09760aa..3b81b1c5 100755 --- a/openstack_election/cmds/check_candidacy.py +++ b/openstack_election/cmds/check_candidacy.py @@ -32,9 +32,11 @@ def main(): parser.add_argument('--tag', dest='tag', default=utils.conf['tag'], help=('The governance tag to validate against. ' 'Default: %(default)s')) + parser.add_argument('-v', '--verbose', action="count", default=0, + help='Increase program verbosity') args = parser.parse_args() - review = utils.get_reviews(args.change_id)[0] + review = utils.get_reviews(args.change_id, verbose=args.verbose)[0] owner = review.get('owner', {}) if args.limit < 0: args.limit = 100 diff --git a/openstack_election/cmds/check_manual.py b/openstack_election/cmds/check_manual.py index 58f575a0..13f15c75 100644 --- a/openstack_election/cmds/check_manual.py +++ b/openstack_election/cmds/check_manual.py @@ -35,6 +35,8 @@ def main(): parser.add_argument('--tag', dest='tag', default=utils.conf['tag'], help=('The governance tag to validate against. ' 'Default: %(default)s')) + parser.add_argument('-v', '--verbose', action="count", default=0, + help='Increase program verbosity') args = parser.parse_args() if args.limit < 0: @@ -48,7 +50,8 @@ def main(): return 1 if check_candidacy.check_candidate(args.project_name, args.email, - projects, limit=args.limit): + projects, limit=args.limit, + verbose=args.verbose): print('SUCCESS: %s is a valid candidate\n\n' % (args.email)) return 0 else: diff --git a/openstack_election/cmds/ci_check_all_candidate_files.py b/openstack_election/cmds/ci_check_all_candidate_files.py index 8d9ef0d9..01b64394 100755 --- a/openstack_election/cmds/ci_check_all_candidate_files.py +++ b/openstack_election/cmds/ci_check_all_candidate_files.py @@ -48,12 +48,12 @@ def validate_filename(filepath): return is_valid -def validate_member(filepath): +def validate_member(filepath, verbose=0): print('Validate email address is OSF member') print('------------------------------------') email = utils.get_email(filepath) - member = utils.lookup_member(email) + member = utils.lookup_member(email, verbose=verbose) is_valid = member.get('data', []) != [] print('Email address: %s %s' % (email, @@ -62,7 +62,7 @@ def validate_member(filepath): return is_valid -def check_for_changes(projects, filepath, limit): +def check_for_changes(projects, filepath, limit, verbose=0): print('Looking for validating changes') print('------------------------------') @@ -71,7 +71,10 @@ def check_for_changes(projects, filepath, limit): project_name = utils.dir2name(project_name, projects) changes_found = check_candidacy.check_candidate(project_name, email, - projects, limit) + projects, limit, + verbose=verbose) + print('Email address: %s %s' % ( + email, {True: 'PASS', False: 'FAIL'}[changes_found])) print('') return bool(changes_found) @@ -113,6 +116,8 @@ def main(): parser.add_argument('files', nargs='*', help='Candidate files to validate.') + parser.add_argument('-v', '--verbose', action="count", default=0, + help='Increase program verbosity') args = parser.parse_args() errors = False @@ -146,13 +151,14 @@ def main(): candidate_ok = True candidate_ok &= validate_filename(filepath) - candidate_ok &= validate_member(filepath) + candidate_ok &= validate_member(filepath, verbose=args.verbose) if candidate_ok: if (election_type == 'ptl' or (election_type == 'combined' and team != 'TC')): candidate_ok &= check_for_changes(projects, filepath, - args.limit) + args.limit, + verbose=args.verbose) errors |= not candidate_ok diff --git a/openstack_election/cmds/template_emails.py b/openstack_election/cmds/template_emails.py index dc43b00d..56baa702 100644 --- a/openstack_election/cmds/template_emails.py +++ b/openstack_election/cmds/template_emails.py @@ -12,7 +12,7 @@ from openstack_election import utils conf = config.load_conf() -REFERENCE_URL = '%s?id=%s' % (utils.PROJECTS_URL, conf['tag']) +REFERENCE_URL = utils.PROJECTS_URL % '/'.join(('tag', conf['tag'])) LEADERLESS_URL = ('https://governance.openstack.org/resolutions/' '20141128-elections-process-for-leaderless-programs.html') diff --git a/openstack_election/owners.py b/openstack_election/owners.py index abe640e7..ffd679f5 100644 --- a/openstack_election/owners.py +++ b/openstack_election/owners.py @@ -162,7 +162,7 @@ def main(options): elif 'ref' in config: ref = config['ref'] else: - ref = 'refs/heads/master' + ref = 'branch/master' # Gerrit change query additions if options.sieve: @@ -184,9 +184,8 @@ def main(options): if projects_file: gov_projects = utils.load_yaml(open(projects_file).read()) else: - gov_projects = utils.get_from_cgit('openstack/governance', - 'reference/projects.yaml', - {'h': ref}) + gov_projects = utils.get_from_git('openstack/governance', + '%s/reference/projects.yaml' % ref) # The set of retired or removed "legacy" projects from governance # are merged into the main dict if their retired-on date falls @@ -196,9 +195,8 @@ def main(options): elif projects_file: old_projects = [] else: - old_projects = utils.get_from_cgit('openstack/governance', - 'reference/legacy.yaml', - {'h': ref}) + old_projects = utils.get_from_git('openstack/governance', + '%s/reference/legacy.yaml' % ref) for project in old_projects: for deliverable in old_projects[project]['deliverables']: retired = old_projects[project]['deliverables'][deliverable].get( diff --git a/openstack_election/templates/tc_campaigning_kickoff.j2 b/openstack_election/templates/tc_campaigning_kickoff.j2 index 204a097c..97d2b20d 100644 --- a/openstack_election/templates/tc_campaigning_kickoff.j2 +++ b/openstack_election/templates/tc_campaigning_kickoff.j2 @@ -21,4 +21,4 @@ concern, and the electorate has the best information to determine the ideal TC composition to address these and other issues that may arise. [1] https://governance.openstack.org/election/ -[2] https://git.openstack.org/cgit/openstack/election/tree/candidates/{{ release }}/TC +[2] https://opendev.org/openstack/election/src/branch/master/candidates/{{ release }}/TC diff --git a/openstack_election/utils.py b/openstack_election/utils.py index 39cf3a35..f344c95a 100644 --- a/openstack_election/utils.py +++ b/openstack_election/utils.py @@ -34,24 +34,26 @@ from openstack_election import exception # Library constants CANDIDATE_PATH = 'candidates' -GERRIT_BASE = 'https://review.openstack.org' +GERRIT_BASE = 'https://review.opendev.org' ELECTION_REPO = 'openstack/election' -CGIT_URL = 'https://git.openstack.org/cgit' -PROJECTS_URL = ('%s/openstack/governance/plain/reference/projects.yaml' % - (CGIT_URL)) +GIT_URL = 'https://opendev.org/' +PROJECTS_URL = GIT_URL + 'openstack/governance/raw/%s/reference/projects.yaml' conf = config.load_conf() exceptions = None # Generic functions -def requester(url, params={}, headers={}): +def requester(url, params={}, headers={}, verbose=0): """A requests wrapper to consistently retry HTTPS queries""" # Try up to 3 times retry = requests.Session() retry.mount("https://", requests.adapters.HTTPAdapter(max_retries=3)) - return retry.get(url=url, params=params, headers=headers) + raw = retry.get(url=url, params=params, headers=headers) + if verbose >= 2: + print("Queried: %s" % raw.url) + return raw def decode_json(raw): @@ -74,43 +76,41 @@ def decode_json(raw): return decoded -def query_gerrit(method, params={}): +def query_gerrit(method, params={}, verbose=0): """Query the Gerrit REST API""" - # The base URL to Gerrit REST API - GERRIT_API_URL = 'https://review.openstack.org/' - - raw = requester(GERRIT_API_URL + method, params=params, - headers={'Accept': 'application/json'}) - return decode_json(raw) + return decode_json(requester("%s/%s" % (GERRIT_BASE, method), + params=params, + headers={'Accept': 'application/json'}, + verbose=verbose)) def load_yaml(yaml_stream): - """Retrieve a file from the cgit interface""" + """Wrapper to load and return YAML data""" return yaml.safe_load(yaml_stream) -def get_from_cgit(project, obj, params={}): - """Retrieve a file from the cgit interface""" +def get_from_git(project, obj, params={}, verbose=0): + """Retrieve a file from the Gitea interface""" - url = 'https://git.openstack.org/cgit/' + project + '/plain/' + obj - raw = requester(url, params=params, - headers={'Accept': 'application/json'}) - return load_yaml(raw.text) + url = "%s%s/raw/%s" % (GIT_URL, project, obj) + return load_yaml(requester(url, params=params, + headers={'Accept': 'application/json'}, + verbose=verbose).text) def get_series_data(): - return get_from_cgit('openstack/releases', - 'deliverables/series_status.yaml') + return get_from_git('openstack/releases', + 'branch/master/deliverables/series_status.yaml') def get_schedule_data(series): - return get_from_cgit('openstack/releases', - 'doc/source/%s/schedule.yaml' % (series)) + return get_from_git('openstack/releases', + 'branch/master/doc/source/%s/schedule.yaml' % (series)) -def lookup_member(email): +def lookup_member(email, verbose=0): """A requests wrapper to querying the OSF member directory API""" # The OpenStack foundation member directory lookup API endpoint @@ -123,9 +123,17 @@ def lookup_member(email): 'email==' + email, ]}, headers={'Accept': 'application/json'}, + verbose=verbose, ) + result = decode_json(raw) - return decode_json(raw) + # Print the profile if verbosity is 1 or higher + if verbose >= 1 and result['data']: + print("Found: " + "https://openstack.org/community/members/profile/%s" + % result['data'][0]['id']) + + return result def load_exceptions(): @@ -150,12 +158,13 @@ def gerrit_datetime(dt): # TODO(tonyb): this is now basically a duplicate of query_gerrit() -def gerrit_query(url, params=None): - r = requester(url, params=params) +def gerrit_query(url, params=None, verbose=0): + r = requester(url, params=params, verbose=verbose) if r.status_code == 200: data = json.loads(r.text[4:]) else: data = [] + return data @@ -199,12 +208,12 @@ def get_fullname(member, filepath=None): return full_name -def get_reviews(query): +def get_reviews(query, verbose=0): opts = ['CURRENT_REVISION', 'CURRENT_FILES', 'DETAILED_ACCOUNTS'] opts_str = '&o=%s' % ('&o='.join(opts)) url = ('%s/changes/?q=%s%s' % (GERRIT_BASE, quote_plus(query, safe='/:=><^.*'), opts_str)) - return gerrit_query(url) + return gerrit_query(url, verbose=verbose) def candidate_files(review): @@ -222,12 +231,12 @@ def check_atc_date(atc): def _get_projects(tag=None): - url = PROJECTS_URL - cache_file = '.projects.pkl' - if tag: - url += '?h=%s' % tag + url = PROJECTS_URL % '/'.join(('tag', tag)) cache_file = '.projects.%s.pkl' % tag + else: + url = PROJECTS_URL % 'branch/master' + cache_file = '.projects.pkl' # Refresh the cache if it's not there or if it's older than a week if (not os.path.isfile(cache_file) or @@ -333,8 +342,8 @@ def build_candidates_list(election=conf['release']): raise exception.MemberNotFoundException(email=email) candidates_lists[project].append({ - 'url': ('%s/%s/plain/%s' % - (CGIT_URL, ELECTION_REPO, + 'url': ('%s%s/raw/branch/master/%s' % + (GIT_URL, ELECTION_REPO, quote_plus(filepath, safe='/'))), 'email': email, 'ircname': get_irc(member), diff --git a/tox.ini b/tox.ini index cec28c3b..cf0faf32 100644 --- a/tox.ini +++ b/tox.ini @@ -27,7 +27,7 @@ commands = {posargs} commands = sphinx-build -v -W -b html -d doc/build/doctrees doc/source doc/build/html [testenv:ci-checks-review] -commands = ci-check-all-candidate-files {posargs:--HEAD} +commands = ci-check-all-candidate-files -v -v {posargs:--HEAD} [testenv:ci-checks-election] commands = ci-check-all-candidate-files