Hardware Certification Tools

Merge lp://qastaging/~brendan-donegan/hwcert-tools/batch_query_fix into lp://qastaging/~hardware-certification/hwcert-tools/reporting-tools

  1. batch_query_fix
  2. Merge into reporting-tools
Proposed by Brendan Donegan
Status: Merged
Approved by: Daniel Manrique
Approved revision: 131
Merged at revision: 129
Proposed branch: lp://qastaging/~brendan-donegan/hwcert-tools/batch_query_fix
Merge into: lp://qastaging/~hardware-certification/hwcert-tools/reporting-tools
Diff against target: 95 lines (+66/-12)
1 file modified
certification_reports/api_utils.py (+66/-12)
To merge this branch: bzr merge lp://qastaging/~brendan-donegan/hwcert-tools/batch_query_fix
Reviewer Review Type Date Requested Status
Daniel Manrique (community) Approve
Brendan Donegan (community) Needs Resubmitting
Review via email: mp+195971@code.qastaging.launchpad.net

Description of the change

Miscellaneous improvements to batch_query. I'm trying to avoid the situation where a query doesn't return any results and the script just crashes. Because my scripts run on cron I'd like to mask this situation. This branch also shortens the code required for batch query with a bit of clever logic. There's still a couple of outstanding issues - one is that the code does nothing when an exception is raised, but should perhaps do some logging. The other is the question whether the failure of a query should raise an exception or just be ignored.

To post a comment you must log in.
Revision history for this message
Daniel Manrique (roadmr) wrote :

Silently ignoring exceptions is bad (tm). I suggest at least logging the exception's error message. The merge request for checkbox-message-get-c4 has an example of how to do that.

Other than that this look awesome and will work well on both python2 and python3 which rocks!

review: Needs Fixing
Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

Indeed, logs should be available in the cron email

lp://qastaging/~brendan-donegan/hwcert-tools/batch_query_fix updated
130. By Brendan Donegan

Add logging when a query fails and raise a QueryError if the query doesn't work even after retrying.

Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

Ok, i made some updates.

review: Needs Resubmitting
Revision history for this message
Daniel Manrique (roadmr) wrote :

Maybe I'm doing something wrong, but:

$ certification_reports/certification_blockers.py -l montreal-lab bugs 12.04.2 roadmr [my secret key] >/tmp/pla.html
Traceback (most recent call last):
  File "certification_reports/certification_blockers.py", line 332, in <module>
    sys.exit(main())
  File "certification_reports/certification_blockers.py", line 324, in main
    api, launchpad, args.enabled)
  File "certification_reports/certification_blockers.py", line 212, in get_blocking_bugs
    certs = get_certificates(url, release, credentials, api, enabled)
  File "certification_reports/certification_blockers.py", line 95, in get_certificates
    return api.batch_query(request_url, params=failed_certs_params)
  File "/media/bulk/src/hwcert-tools/bzr/reporting-tools/certification_reports/api_utils.py", line 33, in batch_query
    args[0] = self.instance_uri + next_query
TypeError: 'tuple' object does not support item assignment

review: Needs Fixing
Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

Urgh, my own testing didn't hit that scenario - whoops, probably that needs to be:

args = (self.instance_uri + next_query,) + args[:-1]

Fixing...

lp://qastaging/~brendan-donegan/hwcert-tools/batch_query_fix updated
131. By Brendan Donegan

Fix modification of args as it was trying to modify a tuple in place.

Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

Okay, fixed and testing with the same command as above and it worked ok

review: Needs Resubmitting
Revision history for this message
Daniel Manrique (roadmr) wrote :

OK, let's approve this then, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches