Code review comment for lp:~brian-murray/daisy/src-version-buckets

Revision history for this message
Evan (ev) wrote :

Mostly looks good. Feel free to merge once you've addressed the points
below.

On Tue, Mar 26, 2013 at 9:57 PM, Brian Murray <email address hidden> wrote:
>
> + oopses.update_source_version_buckets(oops_config, src_package,
> + version, crash_signature)
>

Can you make this a if hasattr(oopses, 'update_source_version_buckets') so
that we don't crash on processing new crashes while we're updating
oops-repository and daisy on production.

> if version:
> oopses.update_bucket_versions(oops_config, crash_signature,
> version)
>
>
> +for key, oops in oops_cf.get_range(columns=cols):
> + count += 1
> + if count % 10000 == 0:
> + break
>

I suspect you had this in for debugging?

> + release = oops['DistroRelease'].encode('utf8')
>

oops.get. You're not guaranteed to get rows with all the column names you
specified.

> +
> + if not release.startswith('Ubuntu '):
> + continue
> + package_data = oops['Package'].split(' ')

+ if len(package_data) < 2:
> + continue
> + version = package_data[1]
>

Please use daisy.utils.split_package_and_version instead, as it catches
some corner cases (hopefully).

+ src_package = oops['SourcePackage']
>

oops.get.

> + srcversbucketsinsert((src_package, version), {oops_id : ''})
>

Typo :)

« Back to merge proposal