I don't really know that code or its purpose, but I've put you an inline
comment anyway.
Diff comments:
> diff --git a/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
> b/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
> index 6362803..8c625ad 100755
> --- a/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
> +++ b/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
> @@ -96,6 +98,20 @@ def finish_job(jobfile, params, code, log_url):
> os.unlink(jobfile)
>
>
> +def check_time_diff(object_time, job_time):
> + splitted_url = object_time.split("/")
> + # date and time of object embedded in swift object path
> + timestamp = splitted_url[4]
> + # last part of the timestamp is a storage ID which we don't need
> + timestamp = "_".join(timestamp.split("_")[0:2])
> + timestamp = datetime.datetime.strptime(timestamp, "%Y%m%d_%H%M%S")
> + diff = abs(timestamp.timestamp() - job_time) / DAY_IN_SECONDS
> + # 15 days either side
> + if diff < 15:
I don't mind hardcoding some values from time to time, but two things here:
* it's usually better to put that as a global var with good naming, a bit like
you did with DAY_IN_SECONDS
* putting that test directly in that function seems odd to me: I would rather
expect a more generic function returning the time difference, and then doing
the comparison when calling that function. You could do for example `if
get_absolute_time_diff(...) > MAX_DAY_DIFF: continue`
> + return True
> + return False
> +
> +
> def process_job(jobfile):
> try:
> with open(jobfile) as f:
--
https://code.launchpad.net/~andersson123/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/459166
Your team Canonical's Ubuntu QA is requested to review the proposed merge of
~andersson123/autopkgtest-cloud:fix-update-github-jobs into
autopkgtest-cloud:master.
--
Mailing list: https://launchpad.net/~canonical-ubuntu-qa
Post to : [email protected]
Unsubscribe : https://launchpad.net/~canonical-ubuntu-qa
More help : https://help.launchpad.net/ListHelp
