Code review comment for lp:~sinzui/launchpad/prf-flavor-loop

Revision history for this message
Curtis Hovey (sinzui) wrote :

This is my branch to fix the flavor striping and file upload looping rules in
the product release finder.

    lp:~sinzui/launchpad/prf-flavor-loop
    Diff size: 117
    Launchpad bug: https://bugs.launchpad.net/bugs/456525
                   https://bugs.launchpad.net/bugs/415595
    Test command: ./bin/test -vv -t "test_prf_finder"
    Pre-implementation: dreamcat4 and suraia, both of whom read the source
                        code with me as we diagnosed why their files were
                        missing.
    Target release: 3.1.10

= Fix the flavor striping and looping rules in the product release finder =

Bug #456525 [PRF does not strip the flavor info after the ~]
    The releases version are incorrect because the ~ that marked the start of
    the flavor information was converted to a -.

Bug #415595 [PRF should import all files]
    For example, I have sushi-1.0.1.tar.bz2 and sushi-1.0.1.tar.gz, but the
    release finder only imports the .tar.bz2. Looking at the code it seems
    that only the first matching file is added.

== Rules ==

Bug #456525 [PRF does not strip the flavor info after the ~]
    Add the '~' to the flavor_pattern.

Bug #415595 [PRF should import all files]
    hasReleaseTarball() wrongly assumes that if the release has anything like
    a code tarball, that there all the downloading is complete. So downloading
    for a release stops once the first package is downloaded which means
    release notes and installers are also being ignored.

    handleRelease() knows the filename. It should pass it to
    hasReleaseTarball() to verify if the release already has the file. Rename
    hasReleaseTarball => hasReleaseFile(self, product_name, series_name,
    release_name, file_name)

== QA ==

    * Run the PRF
    * Visit project php-fpm
    * Verify the 0.6 release exists and it has php-fpm-0.6~5.3.1.tar.gz
    * Visit /sushi.nigiri/1.0
    * Visit several of the release.
    * Verify that each has more than one milestone.

== Lint ==

Linting changed files:
  lib/lp/registry/scripts/productreleasefinder/finder.py
  lib/lp/registry/tests/test_prf_finder.py

== Test ==

    * lib/lp/registry/tests/test_prf_finder.py
      * Added a test to verify the file and an alternate file are handled
        correctly before and after one it uploaded.
      * Added a test to verify that the release.version (AKA milestone.name)
        does not contain the flavor information from the '~'.

== Implementation ==

    * lib/lp/registry/scripts/productreleasefinder/finder.py
      * Updated the flavor_pattern rules to match '~'.
      * Renamed hasReleaseTarball => hasReleaseFile and added the filename
        argument. The method now matches on file name, not file tpe so that
        a tar.gz and tar.bzr will both be uploded. This will also allow
        multiple tar.gz that contain flavor info in the name to be uploaded.

« Back to merge proposal