On Wed, 2015-02-18 at 05:33 -0500, Josef Skladanka wrote:
> Adam,
>
> the run_all code does not really make much sense to me, to be
> honest. After some minor cleanup, the code looks like this:
>
> 171 def run_all(args, wiki=None):
> 172 """Do everything we can: test both Rawhide and Branched
> nightlies
> 173 if they exist, and test current compose if it's different
> from 174 either and it's new.
> 175 """
> 176 skip = None
> 177 (jobs, currev) = jobs_from_current(wiki)
> 178 print("Jobs from current validation event:
> {0}".format(jobs)) 179
> 180 yesterday = datetime.datetime.utcnow() -
> datetime.timedelta(days=1)
> 181 if currev and currev.compose ==
> yesterday.strftime('%Y%m%d'): 182 skip = currev.milestone
> 183
> 184 if not skip.lower() == 'rawhide':
> 185 rawhide_ffrel = fedfind.release.get_release(
> 186 release='Rawhide', compose=yesterday)
> 187 rawjobs = jobs_from_fedfind(rawhide_ffrel)
> 188 print("Jobs from {0}: {1}".format(rawhide_ffrel.version,
> rawjobs))
> 189 jobs.extend(rawjobs)
> 190
> 191 if not skip.lower() == 'branched':
> 192 branched_ffrel = fedfind.release.get_release(
> 193 release=currev.release, compose=yesterday)
> 194 branchjobs = jobs_from_fedfind(branched_ffrel)
> 195 print("Jobs from {0}:
> {1}".format(branched_ffrel.version, branchjobs))
> 196 jobs.extend(branchjobs)
> 197
> 198 if jobs:
> 199 report_results(jobs)
> 200 sys.exit()
>
>
> Which on lines:
> 177-178: Runs the OpenQA jobs for "current" event
> 180: Creates a yesterday's date (formerly done on three lines
> in a weird way)
> 181-182: IIUIC checks whether the "current" compose is from
> yesterday, and if so, then sets skip to either Rawhide or Branched
> 184&191: Fails terribly, when the if-clause on 181 was False
> (because skip equals None in that case) => no job results will be
> reported to wiki matrices
Whoops - I've somehow sent an older version of the patch. Which is odd
because I was using the same file to apply on qa-01. I fixed all of
the above in the next rev. Looks like I had copies of the patch lying
in two directories and I sent the wrong one :(
The 'yesterday' thing is intended to be conditional on a --yesterday
parameter (it's basically a hack for the fact that at the time I was
testing it, the 02-18 nightly compose hadn't finished, but I figured
I'd leave it in as an option because you might hit that scenario
again). Though you're right about doing it more efficiently, dunno how
I wound up with it the silly way it was.
In the latest version skip is initially set to ''. :)
> Also, it is kind of non-clear at the first read, that what
> it does is basically "when you should not skip rawhide, run jobs for
> rawhide and do the same for branched".
> I'd much rather see something like `if skip.lower() !=
> 'rawhide':` and with a proper comment
I'd already added the comments in the latest version. I tend to have
'not ==' rather than '!=' as a tic, not sure why, I don't mind
changing it at all.
> I'm not really sure how the whole 181 if clause works, and why is it
> evidently always True, since you have not encountered the error.
I did encounter the error, don't worry ;)
The point of the clause is a scenario that was actually true
yesterday: the case where one of the day's nightly builds is also the
'current validation event'. Yesterday, 22 Branched 20150217 was both
the 'current validation event' and the day's nightly Branched compose.
It would be wasteful to run jobs for it twice. So what that clause
does is check if the 'compose' for the current event is the same date
as the date we're testing nightlies for (utcdate); if it is, we check
what 'milestone' the current event is for and skip running the nightly
jobs for that milestone, since they're probably (really, certainly)
the same thing.
When I was testing it would do this: find that the current event is
'22 Branched 20150217', run against that (or skip if it had already
done so in a previous test), figure out 'utcdate', realize that it was
20150217 - the same as the current event - read out 'Branched' as the
current event's milestone, set 'skip' to 'branched', and therefore not
schedule any 'branchjobs'.
It could print a message when it skips a nightly for this reason, but
you can actually already tell, because it won't print a "Jobs from:"
message for that nightly. Unless it's skipped it will always print
such a message, even when there are no jobs.
> Also, you mention a --yesterday parameter, which I have not really
> found in the code.
Yeah, later version :/
> I pushed the slightly polished code to the repos, so make sure to
> pull :)
Whoops, please don't push stuff it it seems broken, I trust your
review :) I'm attaching a patch on top of your last commit which
should clean things up.
> I'd really love to see some more comments in you code, which is
> using the wikitcms/relval internal attributes and so on (it gets
> somewhat wild in places).
I often go back and add comments at the last step, that's why they're
missing from the version that got sent out. I'm not sure what you mean
by 'internal attributes'; all the bits of wikitcms that I've used in
this code should be considered public. Is there some convention I'm
missing there? I thought the convention was that _ and __ prefixes are
for private stuff. But anyhow, yeah, things like the version
attributes (release, milestone, compose, version) and event.ff_release
are certainly intended to be available for outside use. I'm explicitly
trying to make it so you can throw (release, milestone, compose)
triples around at fedfind and wikitcms and they'll handle it as
gracefully as possible. Especially, you should always be able to call
relval commands on a fedfind-derived release, milestone and compose
and it'll do the right thing (I hacked in handling of the Rawhide
nightly case via release number guessing in wikitcms
get_validation_event()).
> Please have a look at the run_all() method and:
> * make sure that it handles the possible exceptions (please really
> do _not_ use empty except clause, it is the root of all evil)
I kinda skimped handling any possible exceptions from
fedfind.release.get_release() in run_all() because it should always be
calling it in a valid fashion and hence should never hit one;
get_release() doesn't raise exceptions just because the release
doesn't exist, or anything, it only raises exceptions when it's called
in an inherently nonsensical way. But we can add it quite easily.
> * document the if-clause on #181 and what it means
Patch attached which more or less brings it to my intended version,
with !='s and get_release() exception handling added and your cleanups
preserved. sorry for the mixup! Consider the previous version an
example of my personal kinds of silly thinkos that get fixed up when I
first test the code :P
To answer your question from IRC: 'all' will run on a max of three
composes. So the way we have Coconut set up ATM it'll run on a max of
six images: both arches for all three composes.
During the phase where nightlies are getting nominated it should never
do six; on the days a new nightly gets nominated it'll run for it as
'current event' and skip it as 'nightly', on other days it'll skip
'current event' because it's already been done.
After Alpha TC1 it'd run for six images each day we release a new
TC/RC (and both Rawhide and Branched nightly composes succeed). Other
days it'd do a max of four.
Obviously if we add more arches or images to the test suites, this
goes up accordingly.
--
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net
http://www.happyassassin.net
From cee30d27488874bb82dedc00e2fb659e7cd1a063 Mon Sep 17 00:00:00 2001
From: Adam Williamson <[email protected]>
Date: Wed, 18 Feb 2015 09:48:30 -0800
Subject: [PATCH] 'all' sub-command cleanup and bugfix
The patch jskladan applied was an older broken one I sent
accidentally; apologies. This is more or less my intended
version, with some of the cleanups from jskladan preserved
and a couple of his suggestions added (!= instead of not ==,
and a bit of just-in-case exception handling).
---
tools/openqa_trigger/openqa_trigger.py | 62 +++++++++++++++++++++++-----------
1 file changed, 42 insertions(+), 20 deletions(-)
diff --git a/tools/openqa_trigger/openqa_trigger.py b/tools/openqa_trigger/openqa_trigger.py
index a390d34..1234c34 100755
--- a/tools/openqa_trigger/openqa_trigger.py
+++ b/tools/openqa_trigger/openqa_trigger.py
@@ -169,32 +169,49 @@ def run_compose(args, wiki=None):
sys.exit()
def run_all(args, wiki=None):
- """Do everything we can: test both Rawhide and Branched nightlies
- if they exist, and test current compose if it's different from
- either and it's new.
+ """Do everything we can: test current validation event compose if
+ it's new, amd test both Rawhide and Branched nightlies if they
+ exist and aren't the same as the 'current' compose.
"""
- skip = None
+ skip = ''
+
+ # Run for 'current' validation event.
(jobs, currev) = jobs_from_current(wiki)
print("Jobs from current validation event: {0}".format(jobs))
- yesterday = datetime.datetime.utcnow() - datetime.timedelta(days=1)
- if currev and currev.compose == yesterday.strftime('%Y%m%d'):
+ utcdate = datetime.datetime.utcnow()
+ if args.yesterday:
+ utcdate = utcdate - datetime.timedelta(days=1)
+ if currev and currev.compose == utcdate.strftime('%Y%m%d'):
+ # Don't schedule tests for the same compose as both "today's
+ # nightly" and "current validation event"
skip = currev.milestone
- if not skip.lower() == 'rawhide':
- rawhide_ffrel = fedfind.release.get_release(
- release='Rawhide', compose=yesterday)
- rawjobs = jobs_from_fedfind(rawhide_ffrel)
- print("Jobs from {0}: {1}".format(rawhide_ffrel.version, rawjobs))
- jobs.extend(rawjobs)
-
- if not skip.lower() == 'branched':
- branched_ffrel = fedfind.release.get_release(
- release=currev.release, compose=yesterday)
- branchjobs = jobs_from_fedfind(branched_ffrel)
- print("Jobs from {0}: {1}".format(branched_ffrel.version, branchjobs))
- jobs.extend(branchjobs)
+ # Run for day's Rawhide nightly (if not same as current event.)
+ if skip.lower() != 'rawhide':
+ try:
+ rawhide_ffrel = fedfind.release.get_release(
+ release='Rawhide', compose=utcdate)
+ rawjobs = jobs_from_fedfind(rawhide_ffrel)
+ print("Jobs from {0}: {1}".format(rawhide_ffrel.version, rawjobs))
+ jobs.extend(rawjobs)
+ except ValueError as err:
+ print("Rawhide image discovery failed: {0}".format(err))
+ # Run for day's Branched nightly (if not same as current event.)
+ # We must guess a release for Branched, fedfind cannot do so. Best
+ # guess we can make is the same as the 'current' validation event
+ # compose (this is why we have jobs_from_current return currev).
+ if skip.lower() != 'branched':
+ try:
+ branched_ffrel = fedfind.release.get_release(
+ release=currev.release, milestone='Branched', compose=utcdate)
+ branchjobs = jobs_from_fedfind(branched_ffrel)
+ print("Jobs from {0}: {1}".format(branched_ffrel.version,
+ branchjobs))
+ jobs.extend(branchjobs)
+ except ValueError as err:
+ print("Branched image discovery failed: {0}".format(err))
if jobs:
report_results(jobs)
sys.exit()
@@ -241,7 +258,12 @@ if __name__ == "__main__":
parser_all = subparsers.add_parser(
'all', description="Run for the current validation event (if needed) "
- "and today's Rawhide and Branched nightly's (if found).")
+ "and today's Rawhide and Branched nightly's (if found). 'Today' is "
+ "calculated for the UTC time zone, no matter the system timezone.")
+ parser_all.add_argument(
+ '-y', '--yesterday', help="Run on yesterday's nightlies, not today's",
+ required=False, action='store_true')
+ parser_current.set_defaults(func=run_current)
parser_all.add_argument(
'-t', '--test', help=test_help, required=False, action='store_true')
parser_all.set_defaults(func=run_all)
--
2.3.0
_______________________________________________
qa-devel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/qa-devel
