Differential D727

Prettify some of the code
ClosedPublic
Actions

Authored by jskladan on Jan 19 2016, 3:10 PM.

Details

Reviewers
adamwill
garretraziel
Commits
rOPENQAc53db64e6705: Prettify some of the code
Summary

Got rid of some of the passing around of instances of objects.

Test Plan

Try if the code still works as expected.

Diff Detail

Repository
rOPENQA fedora_openqa
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jskladan retitled this revision from to Prettify some of the code.Jan 19 2016, 3:10 PM
jskladan updated this object.
jskladan edited the test plan for this revision. (Show Details)
jskladan added reviewers: garretraziel, adamwill.
adamwill added a comment.Jan 19 2016, 5:42 PM

So, um. OK. +1 to dropping the JSON parsing bits (I noted at the time I added the force stuff that this ought to be possible now). It does result in generating a couple of openQA API hits every half hour (or however often the 'current' timer runs), but that's not really a problem.

+1 to the ff_rel / build change, I think.

I'm +1 to 'officially' making the wikitcms dep mandatory - the intent at some point was for it to be optional so you can do everything but jobs_from_current and result reporting without it, but I don't think this matters much any more (and it's broken at present - in the last re-arrangement we had report import it unconditionally and cli imports report, so the try/except stuff for the import in cli is fairly pointless). If @garretraziel thinks otherwise, speak up :)

I don't really think do_report is any better than report ("report" is a verb, "I reported the results", so what's the point of 'verbifying' it?) but I don't really mind it if you insist. Ditto the uniqueres stuff, I don't mind, sure. I'm not really sure how much of an improvement it is to pass the wiki_url around instead of the instantiated wiki instance - it just leads to duplication of the wiki instantiation in jobs_from_current and report_results - but I don't hate it. +/-0 to all of those.

scheduler/fedora_openqa_schedule/report.py
168

I don't think you really need to jump through hoops to keep wiki_url optional. All the internal users pass wiki or wiki_url (with your changes) now, we don't have any external consumers of this stuff I don't think, and you're changing function signatures willy-nilly everywhere else anyway so we're already throwing compatibility out the window with this change.

So, just make it mandatory and ditch the if clause. And the sadface. :)

scheduler/fedora_openqa_schedule/schedule.py
217

I'm not really sure making this private is an improvement; OK, external consumers can use jobs_from_compose(), but if they happen to already be using fedfind and have a fedfind Release object, wouldn't it make sense to let them use this instead of having to turn it back into a release/milestone/compose for jobs_from_compose? And is renaming it really necessary?

garretraziel added a comment.Jan 20 2016, 9:45 AM
In D727#13821, @adamwill wrote:

I'm +1 to 'officially' making the wikitcms dep mandatory - the intent at some point was for it to be optional so you can do everything but jobs_from_current and result reporting without it, but I don't think this matters much any more (and it's broken at present - in the last re-arrangement we had report import it unconditionally and cli imports report, so the try/except stuff for the import in cli is fairly pointless). If @garretraziel thinks otherwise, speak up :)

I agree.

I'm not really sure how much of an improvement it is to pass the wiki_url around instead of the instantiated wiki instance - it just leads to duplication of the wiki instantiation in jobs_from_current and report_results - but I don't hate it. +/-0 to all of those.

I think that purpose is that if you want to use these functions for example from IPython, you probably don't want to import wikitcms (and look through documentation on how to instantiate its object) before you call that function. So you only pass normal Python objects around.

scheduler/fedora_openqa_schedule/report.py
168

I agree, change function signature and make wiki_url mandatory.

scheduler/fedora_openqa_schedule/schedule.py
217

I think that it's okay - I assume that whenever will somebody want to use scheduler.py as a module, he probably will not have fedfind object at hand. And still, you CAN use functions with undescore at the beginning, if you want.

garretraziel commandeered this revision.Jan 22 2016, 12:55 PM
garretraziel edited reviewers, added: jskladan; removed: garretraziel.
garretraziel updated this revision to Diff 1875.Jan 22 2016, 12:55 PM
  • correcting some errors
jskladan commandeered this revision.Jan 22 2016, 12:57 PM
jskladan edited reviewers, added: garretraziel; removed: jskladan.
jskladan added inline comments.Jan 22 2016, 1:02 PM
scheduler/fedora_openqa_schedule/schedule.py
217

I don't really think that there are any "external customer" to begin with, but I see you point. On the other hand, I firmly believe in "internal closure" of libraries - meaning that externally available methods should not require an argument from external library. On top of that, marking the method with underscore does not make it private, you can still easily import it, it just means that it's not intended to be used from outside the library. Which IMHO is the fact here.

I'll rename i back to _jobs_from_fedfind though, as it probably makes more sense naming-vise.

jskladan updated this revision to Diff 1876.Jan 22 2016, 1:07 PM
  • Some renaming; made wiki_url required arg
garretraziel accepted this revision.Jan 22 2016, 1:11 PM

Looks OK, works.

This revision is now accepted and ready to land.Jan 22 2016, 1:11 PM
Closed by commit rOPENQAc53db64e6705: Prettify some of the code (authored by Josef Skladanka <jskladan@redhat.com>). · Explain WhyJan 22 2016, 1:32 PM
This revision was automatically updated to reflect the committed changes.
adamwill added a comment.Jan 22 2016, 6:05 PM

We don't have any external users at present, but we were explicitly writing with the possibility that there might be some in mind - for e.g. if we wanted to have scheduling done by taskotron.

Revision Contents

Diff 1877

View Options

fedora-openqa-schedule.py

  • This file was moved to scheduler/fedora-openqa-schedule.py.
View Options

fedora_openqa_schedule/__init__.py

  • This file was moved to scheduler/fedora_openqa_schedule/__init__.py.
View Options

fedora_openqa_schedule/cli.py

  • This file was moved to scheduler/fedora_openqa_schedule/cli.py.
View Options

fedora_openqa_schedule/conf_test_suites.py

  • This file was moved to scheduler/fedora_openqa_schedule/conf_test_suites.py.
View Options

fedora_openqa_schedule/config.py

  • This file was moved to scheduler/fedora_openqa_schedule/config.py.
View Options

fedora_openqa_schedule/report.py

  • This file was moved to scheduler/fedora_openqa_schedule/report.py.
View Options

fedora_openqa_schedule/schedule.py

  • This file was moved to scheduler/fedora_openqa_schedule/schedule.py.
View Options

schedule.conf.sample

  • This file was moved to scheduler/schedule.conf.sample.
View Options

scheduler/fedora-openqa-schedule.py

  • This file was moved from fedora-openqa-schedule.py.
The contents of this file were not changed.
View Options

scheduler/fedora_openqa_schedule/__init__.py

  • This file was moved from fedora_openqa_schedule/__init__.py.
The contents of this file were not changed.
View Options

scheduler/fedora_openqa_schedule/cli.py

  • This file was moved from fedora_openqa_schedule/cli.py.
Show All 22 Lines
23 23​
24"""CLI module for fedora-openqa-schedule.""" 24​"""CLI module for fedora-openqa-schedule."""
25 25​
26# Standard libraries 26​# Standard libraries
27import argparse 27​import argparse
28import logging 28​import logging
29import sys 29​import sys
30 30​
31# External dependencies ​
32# We can at least find images and run OpenQA jobs without wikitcms ​
33try: ​
34 import wikitcms.wiki ​
35except ImportError: ​
36 wikitcms = None ​
37 ​
38# Internal dependencies 31​# Internal dependencies
39import fedora_openqa_schedule.schedule as schedule 32​import fedora_openqa_schedule.schedule as schedule
40import fedora_openqa_schedule.report as report 33​import fedora_openqa_schedule.report as report
41from fedora_openqa_schedule.config import CONFIG 34​from fedora_openqa_schedule.config import CONFIG
42 35​
43logger = logging.getLogger(__name__) 36​logger = logging.getLogger(__name__)
44 37​
45### SUB-COMMAND METHODS 38​### SUB-COMMAND METHODS
46 39​
47def command_compose(args, wiki): 40​def command_compose(args, wiki_url):
48 """run OpenQA on a specified compose, optionally reporting 41​ """run OpenQA on a specified compose, optionally reporting
49 results if a matching wikitcms ValidationEvent is found by 42​ results if a matching wikitcms ValidationEvent is found by
50 relval/wikitcms. 43​ relval/wikitcms.
51 """ 44​ """
52 try: 45​ try:
53 (ff_rel, jobs) = schedule.jobs_from_compose( 46​ (build, jobs) = schedule.jobs_from_compose(
54 args.release, args.milestone, args.compose, arches=args.arch, force=args.force, 47​ args.release, args.milestone, args.compose, arches=args.arch, force=args.force,
55 wait=args.wait) 48​ wait=args.wait)
56 49​
57 except ValueError as err: 50​ except ValueError as err:
58 logger.critical("Could not find compose %s %s %s! Error: %s", 51​ logger.critical("Could not find compose %s %s %s! Error: %s",
59 args.release, args.milestone, args.compose, err) 52​ args.release, args.milestone, args.compose, err)
60 sys.exit(1) 53​ sys.exit(1)
61 54​
62 except schedule.TriggerException as err: 55​ except schedule.TriggerException as err:
63 logger.warning("No jobs run! Compose may not exist? %s", err) 56​ logger.warning("No jobs run! Compose may not exist? %s", err)
64 sys.exit(1) 57​ sys.exit(1)
65 58​
66 except schedule.WaitError: 59​ except schedule.WaitError:
67 logger.critical("Waited too long for compose to appear!") 60​ logger.critical("Waited too long for compose to appear!")
68 sys.exit(1) 61​ sys.exit(1)
69 62​
70 if jobs: 63​ if jobs:
71 build = '_'.join((ff_rel.release, ff_rel.milestone, ff_rel.compose)) 64​ report.wait_and_report(wiki_url, build=build, do_report=args.submit)
72 report.wait_and_report(build=build, wiki=wiki, report=args.submit) ​
73 else: 65​ else:
74 msg = "No jobs run!" 66​ msg = "No jobs run!"
75 if not args.force: 67​ if not args.force:
76 msg = "{0} {1}".format( 68​ msg = "{0} {1}".format(
77 msg, "You did not pass --force: maybe jobs exist for all ISOs/flavors") 69​ msg, "You did not pass --force: maybe jobs exist for all ISOs/flavors")
78 logger.warning(msg) 70​ logger.warning(msg)
79 sys.exit(1) 71​ sys.exit(1)
80 72​
81 logger.debug("finished") 73​ logger.debug("finished")
82 sys.exit() 74​ sys.exit()
83 75​
84def command_current(args, wiki): 76​def command_current(args, wiki_url):
85 """run OpenQA for current release validation event, if we have 77​ """run OpenQA for current release validation event, if we have
86 not already done it. 78​ not already done it.
87 """ 79​ """
88 logger.info("running on current release") 80​ logger.info("running on current release")
89 try: 81​ try:
90 (ff_rel, jobs) = schedule.jobs_from_current(wiki) 82​ (build, jobs) = schedule.jobs_from_current(wiki_url)
91 except schedule.TriggerException as e: 83​ except schedule.TriggerException as e:
92 logger.debug("No jobs run: %s", e) 84​ logger.debug("No jobs run: %s", e)
93 sys.exit(1) 85​ sys.exit(1)
94 # wait for jobs to finish and display results 86​ # wait for jobs to finish and display results
95 if jobs: 87​ if jobs:
96 logger.info("waiting for jobs") 88​ logger.info("waiting for jobs")
97 build = '_'.join((ff_rel.release, ff_rel.milestone, ff_rel.compose)) 89​ report.wait_and_report(wiki_url, build=build, do_report=args.submit)
98 report.wait_and_report(build=build, wiki=wiki, report=args.submit) ​
99 logger.debug("finished") 90​ logger.debug("finished")
100 sys.exit() 91​ sys.exit()
101 92​
102def command_report(args, wiki): 93​def command_report(args, wiki_url):
103 """Map a list of openQA job IDs and/or builds to Wikitcms test 94​ """Map a list of openQA job IDs and/or builds to Wikitcms test
104 results, and either display the ResTups for inspection or report 95​ results, and either display the ResTups for inspection or report
105 the results to the wiki. 96​ the results to the wiki.
106 """ 97​ """
107 jobs = [int(job) for job in args.jobs if job.isdigit()] 98​ jobs = [int(job) for job in args.jobs if job.isdigit()]
108 builds = [build for build in args.jobs if not build.isdigit()] 99​ builds = [build for build in args.jobs if not build.isdigit()]
109 try: 100​ try:
110 if jobs: 101​ if jobs:
111 report.wait_and_report( 102​ report.wait_and_report(
112 job_ids=jobs, wiki=wiki, report=args.submit) 103​ wiki_url, job_ids=jobs, do_report=args.submit)
113 if builds: 104​ if builds:
114 for build in builds: 105​ for build in builds:
115 report.wait_and_report( 106​ report.wait_and_report(
116 build=build, wiki=wiki, report=args.submit) 107​ wiki_url, build=build, do_report=args.submit)
117 except report.LoginError: 108​ except report.LoginError:
118 # Module logs for us, no need to dupe it. 109​ # Module logs for us, no need to dupe it.
119 sys.exit(1) 110​ sys.exit(1)
120 111​
121### ARGUMENT PARSING AND SUB-COMMAND INIT 112​### ARGUMENT PARSING AND SUB-COMMAND INIT
122 113​
123def run(): 114​def run():
124 """Read in args, set up logging and wikitcms, and run sub-command 115​ """Read in args, set up logging and wikitcms, and run sub-command
125 function. 116​ function.
126 """ 117​ """
127 args = parse_args() 118​ args = parse_args()
128 loglevel = getattr( 119​ loglevel = getattr(
129 logging, args.log_level.upper(), logging.INFO) 120​ logging, args.log_level.upper(), logging.INFO)
130 if args.log_file: 121​ if args.log_file:
131 logging.basicConfig(format="%(levelname)s:%(name)s:%(asctime)s:%(message)s", 122​ logging.basicConfig(format="%(levelname)s:%(name)s:%(asctime)s:%(message)s",
132 filename=args.log_file, level=loglevel) 123​ filename=args.log_file, level=loglevel)
133 else: 124​ else:
134 logging.basicConfig(level=loglevel) 125​ logging.basicConfig(level=loglevel)
135 126​
136 wiki = None 127​ wiki_url = "fedoraproject.org"
137 if args.test: 128​ if args.test:
138 logger.debug("using test wiki") 129​ logger.debug("using test wiki")
139 if wikitcms: 130​ wiki_url = "stg.fedoraproject.org"
140 wiki = wikitcms.wiki.Wiki(('https', 'stg.fedoraproject.org'), '/w/') ​
141 else: ​
142 logger.warning("wikitcms not found, reporting to wiki disabled") ​
143 else: ​
144 if wikitcms: ​
145 wiki = wikitcms.wiki.Wiki(('https', 'fedoraproject.org'), '/w/') ​
146 else: ​
147 logger.warning("wikitcms not found, reporting to wiki disabled") ​
148 131​
149 args.func(args, wiki) 132​ args.func(args, wiki_url)
150 133​
151def parse_args(): 134​def parse_args():
152 """Parse arguments with argparse.""" 135​ """Parse arguments with argparse."""
153 test_help = "Operate on the staging wiki (for testing)" 136​ test_help = "Operate on the staging wiki (for testing)"
154 parser = argparse.ArgumentParser(description=( 137​ parser = argparse.ArgumentParser(description=(
155 "Run OpenQA tests for a release validation test event.")) 138​ "Run OpenQA tests for a release validation test event."))
156 subparsers = parser.add_subparsers() 139​ subparsers = parser.add_subparsers()
157 140​
▲ Show 20 Lines • Show All 69 Lines • Show Last 20 Lines
View Options

scheduler/fedora_openqa_schedule/conf_test_suites.py

  • This file was moved from fedora_openqa_schedule/conf_test_suites.py.
The contents of this file were not changed.
View Options

scheduler/fedora_openqa_schedule/config.py

  • This file was moved from fedora_openqa_schedule/config.py.
The contents of this file were not changed.
View Options

scheduler/fedora_openqa_schedule/report.py

  • This file was moved from fedora_openqa_schedule/report.py.
Show First 20 Lines • Show All 63 Lines • ▼ Show 20 Line(s)48def _uniqueres_replacements(job, uniqueres):
64 except ValueError: 64​ except ValueError:
65 # the above fails when the flavor is 'universal'. This is just 65​ # the above fails when the flavor is 'universal'. This is just
66 # to avoid crashing, these values should never be used 66​ # to avoid crashing, these values should never be used
67 payload = imagetype = 'universal' 67​ payload = imagetype = 'universal'
68 imagetype = imagetype.replace('boot', 'netinst') 68​ imagetype = imagetype.replace('boot', 'netinst')
69 imagetype = imagetype.replace('dvd', 'offline') 69​ imagetype = imagetype.replace('dvd', 'offline')
70 if payload.lower() == 'generic': 70​ if payload.lower() == 'generic':
71 payload = 'Server' 71​ payload = 'Server'
72 if job['settings'].has_key('UEFI'): 72​ if 'UEFI' in job['settings']:
73 uefi = 'UEFI' 73​ uefi = 'UEFI'
74 bootmethod = 'x86 UEFI' 74​ bootmethod = 'x86 UEFI'
75 else: 75​ else:
76 uefi = arch 76​ uefi = arch
77 bootmethod = 'x86 BIOS' 77​ bootmethod = 'x86 BIOS'
78 78​
79 for key in uniqueres: 79​ changed = {}
80 uniqueres[key] = uniqueres[key].replace('$RUNARCH_OR_UEFI$', uefi) 80​ for key, value in uniqueres.iteritems():
81 uniqueres[key] = uniqueres[key].replace('$FS$', fs) 81​ value = value.replace('$RUNARCH_OR_UEFI$', uefi)
82 uniqueres[key] = uniqueres[key].replace('$RUNARCH$', arch) 82​ value = value.replace('$FS$', fs)
83 uniqueres[key] = uniqueres[key].replace('$BOOTMETHOD$', bootmethod) 83​ value = value.replace('$RUNARCH$', arch)
84 uniqueres[key] = uniqueres[key].replace('$PAYLOAD$', payload) 84​ value = value.replace('$BOOTMETHOD$', bootmethod)
85 uniqueres[key] = uniqueres[key].replace('$IMAGETYPE$', imagetype) 85​ value = value.replace('$PAYLOAD$', payload)
86​ value = value.replace('$IMAGETYPE$', imagetype)
87​ changed[key] = value
88​
89​ return changed
90​
86 91​
87def get_passed_testcases(jobs): 92​def get_passed_testcases(jobs):
88 """Given an iterable of job dicts - any waiting, filtering and so 93​ """Given an iterable of job dicts - any waiting, filtering and so
89 on is assumed to have already happened - returns a list of 94​ on is assumed to have already happened - returns a list of
90 wikitcms ResTuples derived from any passed tests. 95​ wikitcms ResTuples derived from any passed tests.
91 """ 96​ """
92 passed_testcases = set() 97​ passed_testcases = set()
93 for job in jobs: 98​ for job in jobs:
94 if job['result'] == 'passed': 99​ if job['result'] == 'passed':
95 (release, milestone, compose) = job['settings']['BUILD'].split('_') 100​ (release, milestone, compose) = job['settings']['BUILD'].split('_')
96 testsuite = job['settings']['TEST'] 101​ testsuite = job['settings']['TEST']
97 # There usually ought to be an entry in TESTSUITES for all 102​ # There usually ought to be an entry in TESTSUITES for all
98 # tests, but just in case someone messed up, let's be safe 103​ # tests, but just in case someone messed up, let's be safe
99 if not testsuite in conf_test_suites.TESTSUITES: 104​ if testsuite not in conf_test_suites.TESTSUITES:
100 logger.warning("No TESTSUITES entry found for test {0}!".format(testsuite)) 105​ logger.warning("No TESTSUITES entry found for test {0}!".format(testsuite))
101 continue 106​ continue
102 for testcase in conf_test_suites.TESTSUITES[testsuite]: 107​ for testcase in conf_test_suites.TESTSUITES[testsuite]:
103 # each 'testsuite' is a list using testcase names to indicate which Wikitcms tests 108​ # each 'testsuite' is a list using testcase names to indicate which Wikitcms tests
104 # have passed if this job passes. Each testcase name is the name of a dict in the 109​ # have passed if this job passes. Each testcase name is the name of a dict in the
105 # TESTCASES dict-of-dicts which more precisely identifies the 'test instance' (when 110​ # TESTCASES dict-of-dicts which more precisely identifies the 'test instance' (when
106 # there is more than one for a testcase) and environment for which the result 111​ # there is more than one for a testcase) and environment for which the result
107 # should be filed. We do a deepcopy here because otherwise the dict in TESTCASES 112​ # should be filed. We do a deepcopy here because otherwise the dict in TESTCASES
108 # itself is modified in-place and other jobs for the same testcase (but a different 113​ # itself is modified in-place and other jobs for the same testcase (but a different
109 # environment, section or testname) will read the modified values and be messed up. 114​ # environment, section or testname) will read the modified values and be messed up.
110 uniqueres = copy.deepcopy(conf_test_suites.TESTCASES[testcase]) 115​
111 # replace $FOO$ values in uniqueres 116​ # replace $FOO$ values in uniqueres
112 _uniqueres_replacements(job, uniqueres) 117​ uniqueres = _uniqueres_replacements(job, conf_test_suites.TESTCASES[testcase])
113 result = ResTuple( 118​ result = ResTuple(
114 testtype=uniqueres['type'], release=release, milestone=milestone, 119​ testtype=uniqueres['type'], release=release, milestone=milestone,
115 compose=compose, testcase=testcase, section=uniqueres.get('section'), 120​ compose=compose, testcase=testcase, section=uniqueres.get('section'),
116 testname=uniqueres.get('name', ''), env=uniqueres.get('env', ''), 121​ testname=uniqueres.get('name', ''), env=uniqueres.get('env', ''),
117 status='pass', bot=True) 122​ status='pass', bot=True)
118 passed_testcases.add(result) 123​ passed_testcases.add(result)
119 124​
120 return sorted(list(passed_testcases), key=attrgetter('testcase')) 125​ return sorted(list(passed_testcases), key=attrgetter('testcase'))
121 126​
122def wait_and_report(job_ids=None, build=None, wiki=None, report=None, waittime=None): 127​
128​def wait_and_report(wiki_url, job_ids=None, build=None, do_report=None, waittime=None):
123 """Find some openQA jobs and report the results to Wikitcms (via 129​ """Find some openQA jobs and report the results to Wikitcms (via
124 report_results). Either job_ids (an iterable of IDs) or build (an 130​ report_results). Either job_ids (an iterable of IDs) or build (an
125 openQA BUILD string) is required. wiki and report are passed to 131​ openQA BUILD string) is required. wiki_url and do_report are passed to
126 report_results and documented there. waittime (int) is how long to 132​ report_results and documented there. waittime (int) is how long to
127 wait for jobs to complete before giving up; if 'build' is used, 133​ wait for jobs to complete before giving up; if 'build' is used,
128 will also wait for jobs to appear. If jobs are not present and 134​ will also wait for jobs to appear. If jobs are not present and
129 complete when waittime expires, 135​ complete when waittime expires,
130 openqa_client.exceptions.WaitException will be raised. If waittime 136​ openqa_client.exceptions.WaitException will be raised. If waittime
131 is 0, no waiting is done, and if jobs are not complete, the 137​ is 0, no waiting is done, and if jobs are not complete, the
132 exception will raise immediately. 138​ exception will raise immediately.
133 """ 139​ """
134 if not job_ids and not build: 140​ if not job_ids and not build:
135 raise ValueError("wait_and_report requires either job_ids or build.") 141​ raise ValueError("wait_and_report requires either job_ids or build.")
136 142​
137 if waittime is None: 143​ if waittime is None:
138 waittime = CONFIG.getint('report', 'jobs-wait') 144​ waittime = CONFIG.getint('report', 'jobs-wait')
139 # Use the openQA client lib to wait for jobs to be done. Will 145​ # Use the openQA client lib to wait for jobs to be done. Will
140 # raise an openQA client error on wait expiry. 146​ # raise an openQA client error on wait expiry.
141 client = OpenQA_Client() 147​ client = OpenQA_Client()
142 # iterate_jobs can work on jobs or build; whichever is set will be used 148​ # iterate_jobs can work on jobs or build; whichever is set will be used
143 for joblist in client.iterate_jobs(jobs=job_ids, build=build, waittime=waittime): 149​ for joblist in client.iterate_jobs(jobs=job_ids, build=build, waittime=waittime):
144 report_results(joblist, wiki=wiki, report=report) 150​ report_results(joblist, wiki_url, do_report=do_report)
151​
145 152​
146def report_results(jobs, wiki=None, report=None): 153​def report_results(jobs, wiki_url, do_report=None):
147 """Report results from openQA jobs to Wikitcms. jobs is an 154​ """Report results from openQA jobs to Wikitcms. jobs is an
148 iterable of openQA job dicts. If report is False, will just print 155​ iterable of openQA job dicts. If do_report is False, will just print
149 out the python-wikitcms ResTups for inspection. If report is None, 156​ out the python-wikitcms ResTups for inspection. If do_report is None,
150 will read the setting from the config file; if no config file is 157​ will read the setting from the config file; if no config file is
151 present, default is True. 158​ present, default is True.
152 """ 159​ """
153 passed_testcases = get_passed_testcases(jobs) 160​ passed_testcases = get_passed_testcases(jobs)
154 logger.info("passed testcases: %s", passed_testcases) 161​ logger.info("passed testcases: %s", passed_testcases)
155 if report is None: 162​ if do_report is None:
156 report = CONFIG.getboolean('report', 'submit') 163​ do_report = CONFIG.getboolean('report', 'submit')
157 164​
158 if report: 165​ if do_report:
159 logger.info("reporting test passes") 166​ logger.info("reporting test passes")
160 if not wiki: 167​ wiki = Wiki(('https', wiki_url), '/w/')
161 wiki = Wiki() ​
162 if not wiki.logged_in: 168​ if not wiki.logged_in:
adamwillUnsubmitted
Not Done

I don't think you really need to jump through hoops to keep wiki_url optional. All the internal users pass wiki or wiki_url (with your changes) now, we don't have any external consumers of this stuff I don't think, and you're changing function signatures willy-nilly everywhere else anyway so we're already throwing compatibility out the window with this change.

So, just make it mandatory and ditch the if clause. And the sadface. :)

garretrazielUnsubmitted
Not Done

I agree, change function signature and make wiki_url mandatory.

163 wiki.login() 169​ wiki.login()
164 if not wiki.logged_in: 170​ if not wiki.logged_in:
165 logger.error("could not log in to wiki") 171​ logger.error("could not log in to wiki")
166 raise LoginError 172​ raise LoginError
167 173​
168 # Submit the results 174​ # Submit the results
169 (insuffs, dupes) = wiki.report_validation_results(passed_testcases) 175​ (insuffs, dupes) = wiki.report_validation_results(passed_testcases)
170 for dupe in dupes: 176​ for dupe in dupes:
171 tmpl = "already reported result for test %s, env %s! Will not report dupe." 177​ tmpl = "already reported result for test %s, env %s! Will not report dupe."
172 logger.info(tmpl, dupe.testcase, dupe.env) 178​ logger.info(tmpl, dupe.testcase, dupe.env)
179​ for insuff in insuffs:
180​ tmpl = "insufficient data for test %s, env %s! Will not report."
181​ logger.info(tmpl, insuff.testcase, insuff.env)
173 182​
174 else: 183​ else:
175 logger.warning("no reporting is done") 184​ logger.warning("no reporting is done")
View Options

scheduler/fedora_openqa_schedule/schedule.py

  • This file was moved from fedora_openqa_schedule/schedule.py.
Show All 26 Lines
27import json 27​import json
28import logging 28​import logging
29import os.path 29​import os.path
30import re 30​import re
31 31​
32# External dependencies 32​# External dependencies
33import fedfind.helpers 33​import fedfind.helpers
34import fedfind.release 34​import fedfind.release
35​import wikitcms.wiki
35from openqa_client.client import OpenQA_Client 36​from openqa_client.client import OpenQA_Client
36from six.moves import configparser 37​from six.moves import configparser
37from six.moves.urllib.request import urlopen 38​from six.moves.urllib.request import urlopen
38 39​
40​
39# Internal dependencies 41​# Internal dependencies
40from fedora_openqa_schedule.config import CONFIG 42​from fedora_openqa_schedule.config import CONFIG
41 43​
42logger = logging.getLogger(__name__) 44​logger = logging.getLogger(__name__)
43 45​
44 46​
45class TriggerException(Exception): 47​class TriggerException(Exception):
46 pass 48​ pass
47 49​
48 50​
49class WaitError(Exception): 51​class WaitError(Exception):
50 """Raised when we waited for something too long.""" 52​ """Raised when we waited for something too long."""
51 pass 53​ pass
52 54​
53 55​
54def read_last(arches=None): ​
55 """Read last tested version from file. ​
56 If no architectures were provided, load them from config file. ​
57 """ ​
58 persistent = CONFIG.get('schedule', 'persistent') ​
59 if not arches: ​
60 # split arches by words (works for both space- and comma-separated values) ​
61 arches = re.split(r'\W+', CONFIG.get('schedule', 'arches').strip()) ​
62 logger.debug("reading latest checked version from %s", persistent) ​
63 result = {} ​
64 try: ​
65 f = open(persistent, "r") ​
66 json_raw = f.read() ​
67 f.close() ​
68 json_parsed = json.loads(json_raw) ​
69 except IOError: ​
70 logger.warning("cannot read file %s", persistent) ​
71 return result, {} ​
72 ​
73 for arch in arches: ​
74 result[arch] = json_parsed.get(arch, None) ​
75 logger.info("latest version for %s: %s", arch, result[arch]) ​
76 return result, json_parsed ​
77 ​
78 ​
79def download_image(image, iso_path=None): 56​def download_image(image, iso_path=None):
80 """Download a given image with a name that should be unique, 57​ """Download a given image with a name that should be unique,
81 optionally specifying destination directory. 58​ optionally specifying destination directory.
82 Returns the filename of the image (not the path). 59​ Returns the filename of the image (not the path).
83 """ 60​ """
84 # if no iso_path is specified, parse it from config file 61​ # if no iso_path is specified, parse it from config file
85 if not iso_path: 62​ if not iso_path:
86 iso_path = CONFIG.get('schedule', 'iso-path') 63​ iso_path = CONFIG.get('schedule', 'iso-path')
▲ Show 20 Lines • Show All 76 Lines • ▼ Show 20 Line(s)92def run_openqa_jobs(isoname, flavor, arch, build, force=False):
163 140​
164 output = client.openqa_request('POST', 'isos', params) 141​ output = client.openqa_request('POST', 'isos', params)
165 logger.debug("run_openqa_jobs: executed") 142​ logger.debug("run_openqa_jobs: executed")
166 logger.debug("run_openqa_jobs: planned jobs: %s", output["ids"]) 143​ logger.debug("run_openqa_jobs: planned jobs: %s", output["ids"])
167 144​
168 return output["ids"] 145​ return output["ids"]
169 146​
170 147​
171def jobs_from_current(wiki, force=False, arches=None): 148​def jobs_from_current(wiki_url, force=False, arches=None):
172 """Schedule jobs against the 'current' release validation event 149​ """Schedule jobs against the 'current' release validation event
173 (according to wikitcms) if we have not already. Returns the job 150​ (according to wikitcms) if we have not already. Returns the job
174 list. If force is False, for each ISO, jobs will only be scheduled 151​ list. If force is False, for each ISO, jobs will only be scheduled
175 if there are no existing, non-cancelled jobs for the same ISO and 152​ if there are no existing, non-cancelled jobs for the same ISO and
176 flavor. If not set, load list of arches from config file. 153​ flavor. If not set, load list of arches from config file.
177 """ 154​ """
178 if not wiki: ​
179 logger.warning("python-wikitcms is required for current validation event discovery.") ​
180 return ([], None) ​
181 # if arches weren't specified as argument, parse them from config file 155​ # if arches weren't specified as argument, parse them from config file
182 if not arches: 156​ if not arches:
183 arches = re.split(r'\W+', CONFIG.get('schedule', 'arches').strip()) 157​ arches = re.split(r'\W+', CONFIG.get('schedule', 'arches').strip())
184 last_versions, json_parsed = read_last() ​
185 currev = wiki.current_event ​
186 logger.info("current event: %s", currev.version) ​
187 runarches = [] ​
188 for arch in arches: ​
189 last_version = last_versions.get(arch, None) ​
190 if last_version and last_version >= currev.sortname: ​
191 logger.info("skipped: %s: %s is newer or equal to %s", arch, last_version, currev.sortname) ​
192 else: ​
193 runarches.append(arch) ​
194 logger.debug("%s will be tested in version %s", arch, currev.sortname) ​
195 json_parsed[arch] = currev.sortname ​
196 158​
197 jobs = [] 159​ wiki = wikitcms.wiki.Wiki(('https', wiki_url), '/w/')
198 160​
199 if not runarches: 161​ currev = wiki.current_event
200 raise TriggerException("Skipped all arches, nothing to do.") 162​ logger.info("current event: %s", currev.version)
201 163​
202 jobs = jobs_from_fedfind(currev.ff_release, arches=runarches, force=force) 164​ jobs = _jobs_from_fedfind(currev.ff_release, arches=arches, force=force)
203 logger.info("jobs_from_current: planned jobs: %s", ' '.join(str(j) for j in jobs)) 165​ logger.info("jobs_from_current: planned jobs: %s", ' '.join(str(j) for j in jobs))
204 166​
205 # write info about latest versions 167​ build = '_'.join((currev.ff_release.release, currev.ff_release.milestone, currev.ff_release.compose))
206 f = open(CONFIG.get('schedule', 'persistent'), "w") 168​ return (build, jobs)
207 f.write(json.dumps(json_parsed)) ​
208 f.close() ​
209 logger.debug("written info about newest version") ​
210 ​
211 return (currev.ff_release, jobs) ​
212 169​
213 170​
214def jobs_from_compose(release='', milestone='', compose='', arches=None, force=False, wait=None): 171​def jobs_from_compose(release='', milestone='', compose='', arches=None, force=False, wait=None):
215 """Schedule jobs against a specific release/compose. Returns the 172​ """Schedule jobs against a specific release/compose. Returns the
216 list of job IDs. 173​ list of job IDs.
217 174​
218 'release', 'milestone' and 'compose' identify a release according 175​ 'release', 'milestone' and 'compose' identify a release according
219 to fedfind conventions (e.g. '23', 'Beta', 'TC2' or '24', 176​ to fedfind conventions (e.g. '23', 'Beta', 'TC2' or '24',
220 'Branched', '20160301'). A ValueError may be raised (by fedfind) 177​ 'Branched', '20160301'). A ValueError may be raised (by fedfind)
221 if there is something wrong with these values. 178​ if there is something wrong with these values.
222 179​
223 arches may be an iterable of arches to run on. If not specified, 180​ arches may be an iterable of arches to run on. If not specified,
224 the default from config file will be used. 181​ the default from config file will be used.
225 182​
226 If force is False, for each ISO, jobs will only be scheduled if 183​ If force is False, for each ISO, jobs will only be scheduled if
227 there are no existing, non-cancelled jobs for the same ISO and 184​ there are no existing, non-cancelled jobs for the same ISO and
228 flavor. 185​ flavor.
229 186​
230 'wait' is a number of minutes to wait for the compose to appear 187​ 'wait' is a number of minutes to wait for the compose to appear
231 before scheduling the jobs; if None, the value will be read from 188​ before scheduling the jobs; if None, the value will be read from
232 configuration (see config.py for default). Will raise WaitError if 189​ configuration (see config.py for default). Will raise WaitError if
233 the compose cannot be found after the wait period. If wait is 0, 190​ the compose cannot be found after the wait period. If wait is 0,
234 jobs_from_fedfind will raise TriggerException if the compose does 191​ _jobs_from_fedfind will raise TriggerException if the compose does
235 not exist. 192​ not exist.
236 """ 193​ """
237 # Get the fedfind release object. 194​ # Get the fedfind release object.
238 logger.debug("jobs_from_compose: querying fedfind for compose: %s %s %s", release, 195​ logger.debug("jobs_from_compose: querying fedfind for compose: %s %s %s", release,
239 milestone, compose) 196​ milestone, compose)
240 ff_release = fedfind.release.get_release(release=release, milestone=milestone, 197​ ff_release = fedfind.release.get_release(release=release, milestone=milestone,
241 compose=compose) 198​ compose=compose)
242 logger.info("jobs_from_compose: running on compose: %s", ff_release.version) 199​ logger.info("jobs_from_compose: running on compose: %s", ff_release.version)
243 200​
244 if wait is None: 201​ if wait is None:
245 wait = CONFIG.getint('schedule', 'compose-wait') 202​ wait = CONFIG.getint('schedule', 'compose-wait')
246 if wait: 203​ if wait:
247 logger.info("jobs_from_compose: Waiting up to %s mins for compose", str(wait)) 204​ logger.info("jobs_from_compose: Waiting up to %s mins for compose", str(wait))
248 try: 205​ try:
249 ff_release.wait(waittime=wait) 206​ ff_release.wait(waittime=wait)
250 except fedfind.exceptions.WaitError as err: 207​ except fedfind.exceptions.WaitError as err:
251 raise WaitError(err) 208​ raise WaitError(err)
252 209​
253 jobs = jobs_from_fedfind(ff_release, arches=arches, force=force) 210​ jobs = _jobs_from_fedfind(ff_release, arches=arches, force=force)
254 logger.info("jobs_from_compose: planned jobs: %s", ' '.join(str(j) for j in jobs)) 211​ logger.info("jobs_from_compose: planned jobs: %s", ' '.join(str(j) for j in jobs))
255 return (ff_release, jobs) 212​
213​ build = '_'.join((ff_release.release, ff_release.milestone, ff_release.compose))
214​ return (build, jobs)
256 215​
257 216​
258def jobs_from_fedfind(ff_release, force=False, arches=None, imagetypes=None, payloads=None): 217​def _jobs_from_fedfind(ff_release, force=False, arches=None, imagetypes=None, payloads=None):
adamwillUnsubmitted
Not Done

I'm not really sure making this private is an improvement; OK, external consumers can use jobs_from_compose(), but if they happen to already be using fedfind and have a fedfind Release object, wouldn't it make sense to let them use this instead of having to turn it back into a release/milestone/compose for jobs_from_compose? And is renaming it really necessary?

garretrazielUnsubmitted
Not Done

I think that it's okay - I assume that whenever will somebody want to use scheduler.py as a module, he probably will not have fedfind object at hand. And still, you CAN use functions with undescore at the beginning, if you want.

jskladanAuthorUnsubmitted
Not Done

I don't really think that there are any "external customer" to begin with, but I see you point. On the other hand, I firmly believe in "internal closure" of libraries - meaning that externally available methods should not require an argument from external library. On top of that, marking the method with underscore does not make it private, you can still easily import it, it just means that it's not intended to be used from outside the library. Which IMHO is the fact here.

I'll rename i back to _jobs_from_fedfind though, as it probably makes more sense naming-vise.

259 """Given a fedfind.Release object, find the ISOs we want and run 218​ """Given a fedfind.Release object, find the ISOs we want and run
260 jobs on them. arches is an iterable of arches to run on, if not 219​ jobs on them. arches is an iterable of arches to run on, if not
261 specified, we'll use value from config file. If force is False, 220​ specified, we'll use value from config file. If force is False,
262 for each ISO, jobs will only be scheduled if there are no 221​ for each ISO, jobs will only be scheduled if there are no
263 existing, non-cancelled jobs for the same ISO and flavor. 222​ existing, non-cancelled jobs for the same ISO and flavor.
264 """ 223​ """
265 # if arches, imagetypes or payload weren't specified as argument, parse them from config file 224​ # if arches, imagetypes or payload weren't specified as argument, parse them from config file
266 if not arches: 225​ if not arches:
▲ Show 20 Lines • Show All 77 Lines • Show Last 20 Lines
View Options

scheduler/schedule.conf.sample

  • This file was moved from schedule.conf.sample.
The contents of this file were not changed.
View Options

scheduler/setup.py

  • This file was moved from setup.py.
The contents of this file were not changed.
View Options

setup.py

  • This file was moved to scheduler/setup.py.