Merge lp://qastaging/~brendan-donegan/hwcert-tools/datacentres_api_update into lp://qastaging/~hardware-certification/hwcert-tools/reporting-tools
Proposed by
Brendan Donegan
| Status: | Merged |
|---|---|
| Approved by: | Daniel Manrique |
| Approved revision: | 125 |
| Merged at revision: | 126 |
| Proposed branch: | lp://qastaging/~brendan-donegan/hwcert-tools/datacentres_api_update |
| Merge into: | lp://qastaging/~hardware-certification/hwcert-tools/reporting-tools |
| Diff against target: |
43 lines (+22/-8) 1 file modified
certification_reports/certification_report.py (+22/-8) |
| To merge this branch: | bzr merge lp://qastaging/~brendan-donegan/hwcert-tools/datacentres_api_update |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Daniel Manrique (community) | Approve | ||
|
Review via email:
|
|||
Description of the change
The datacentres API of c3 has been changed so that hosts are 'linked' by their id rather than being embedded in the datacentre. This means we need to do extra queries to get the host itself in order to determine the canonical_id for the purpose of mapping CIDs to datacentres.
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
result.json can be an attribute or a method, depending on the version of requests that you're using. This tripped me up a bit; I think the hasattr code block is meant to handle it but I can tell you it doesn't work for that, it crashed for me until I changed all results.json instances in the script to use the method rather than the attribute.
Once I got it running, it took several minutes. Maybe later you'll want to use session rather than plain requests to speed things up a bit and reduce some more load on the server.
Also, running flake8 on the script spits out about 20 pep8 noncompliances, which may be good to solve at some point. They weren't introduced by this patch so we can handle that later if desired.
As long as this runs OK with the version of requests that's on the satellites I'm OK with it.