Merge lp://staging/~brendan-donegan/checkbox-certification/return_status_url into lp://staging/checkbox-certification
Proposed by
Brendan Donegan
Status: | Merged |
---|---|
Approved by: | Daniel Manrique |
Approved revision: | 578 |
Merged at revision: | 577 |
Proposed branch: | lp://staging/~brendan-donegan/checkbox-certification/return_status_url |
Merge into: | lp://staging/checkbox-certification |
Diff against target: |
46 lines (+17/-2) 2 files modified
bin/checkbox-certification-submit (+14/-0) debian/changelog (+3/-2) |
To merge this branch: | bzr merge lp://staging/~brendan-donegan/checkbox-certification/return_status_url |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Daniel Manrique (community) | Approve | ||
Brendan Donegan (community) | Needs Resubmitting | ||
Review via email: mp+158623@code.staging.launchpad.net |
Description of the change
HEXR now has a feature to return a url which leads to a page showing the status of a submission (e.g. processing/
To post a comment you must log in.
One thing to fix:
25 + logging. info("Submissio n status: " + base_url + status['url'])
Best to use logging's string interpolation stuff here:
logging. info("Submissio n status: %s%s", base_url, status['url'])
Also, a couple of comments.
Do you want to handle the situation in which the server doesn't return json (in which case json.loads will throw an exception), and the situation in which the json is *not* what you expect (status['url'] will throw KeyError)?
Then, a minor nitpick:
17 + split_url = args.rest_ transport. split(' //') [split_ url[0], 1].split( '/')[0] ])
18 + base_url = '//'.join(
19 + split_url[
This works, but looks a bit crazy, I suggest the following code, which is also only 3 lines (including import) and IMHO clearer in what it does:
import urllib.parse parse.urlparse( args.rest_ transport) parse.urlunpars e((url_ parts.scheme, url_parts.netloc, "", "", ""))
url_parts = urllib.
base_url = urllib.
Your code works fine, however, so I'm cool either way.