Merge ~alexmurray/ubuntu-qa-tools:unembargo-warn-security-updates-on-fridays into ubuntu-qa-tools:master

Proposed by Alex Murray
Status: Merged
Merged at revision: 37630020b56978816a43b01b069be0899d46a8b3
Proposed branch: ~alexmurray/ubuntu-qa-tools:unembargo-warn-security-updates-on-fridays
Merge into: ubuntu-qa-tools:master
Diff against target: 22 lines (+11/-0)
1 file modified
security-tools/unembargo (+11/-0)
Reviewer Review Type Date Requested Status
Steve Beattie Approve
Alex Murray Needs Fixing
Review via email:
To post a comment you must log in.
Revision history for this message
Alex Murray (alexmurray) wrote :

Hmm this should probably also handle ESM PPAs etc.

review: Needs Fixing
Revision history for this message
Alex Murray (alexmurray) wrote :

Rebased the original commit on top of master and added support for checking Friday for ESM PPAs as well as the security pocket.

Revision history for this message
Steve Beattie (sbeattie) wrote :

This looks good to me, approving.

Extremely minor nits that absolutely do not block merging would be:

- use of magic numbers; it'd be nice not to have to remember that 4 == friday; not sure if the python libraries have a constant defined for this.

- a lot of our scripts do a lot o stuff inline, resuliting in long code sequences that an be overwhelming. This makes things slightly harder to understand and harder to write unit tests for. I don't necessarily think this added check is worth bothering with a separate function (expecially because most of the added bits is handling what to do if the check trips), but it's worth thinking about as we add things.

- if you're going to rebase or add commits after you've gone ahead and made a merge request, it's nice to add a reference link to the merge request in your commit message.

Thanks for the nice improvement!

review: Approve
Revision history for this message
Alex Murray (alexmurray) wrote :

Regarding the magic number - I can't find anything like an enum which defines this - but we could calculate it as:

time.strptime("Fri", "%a").tm_wday

although this seems more unwieldy to me.

Thanks for the review - I'll add a link in the merge-commit message to this. Cheers.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/security-tools/unembargo b/security-tools/unembargo
2index 73595e7..a78c826 100755
3--- a/security-tools/unembargo
4+++ b/security-tools/unembargo
5@@ -137,6 +137,17 @@ def pending_milestone(ubuntu, release):
6 return milestone
9+# don't release security updates on Fridays
10+if (opt.pocket == SECURITY_POCKET or opt.esm or opt.esm_apps or opt.esm_infra) and \
11+ == 4:
12+ print("WARNING: The security team has a policy of not releasing security updates "
13+ "on Fridays.")
14+ if not opt.force:
15+ print("NOTE: To override this check and publish anyway please use the "
16+ "--force.")
17+ sys.exit(1)
18+ else:
19+ print("NOTE: Forcing the release of security updates on Fridays.")
21 print("Loading Ubuntu Distribution ...")
22 lp_version = "devel"


People subscribed via source and target branches